diff --git a/src/openforms/analytics_tools/migrations/0003_cspsetting_identifier.py b/src/openforms/analytics_tools/migrations/0003_cspsetting_identifier.py new file mode 100644 index 0000000000..abd6a92e59 --- /dev/null +++ b/src/openforms/analytics_tools/migrations/0003_cspsetting_identifier.py @@ -0,0 +1,78 @@ +# Generated by Django 3.2.21 on 2023-11-22 17:27 + +from django.db import migrations +from django.db.migrations.state import StateApps +from django.db.backends.base.schema import BaseDatabaseSchemaEditor + +from openforms.analytics_tools.constants import AnalyticsTools + +SITEIMPROVE_VALUES = [ + "https://siteimproveanalytics.com", + "https://siteimproveanalytics.com", + "https://*.siteimproveanalytics.io", +] +GA_VALUES = ["https://www.googleanalytics.com", "https://www.googletagmanager.com"] + +FIELD_TO_IDENTIFIER = { + "matomo_url": AnalyticsTools.matomo, + "piwik_pro_url": AnalyticsTools.piwik_pro, + "piwik_url": AnalyticsTools.piwik, +} + + +def set_identifier(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None: + """Set the corresponding analytic tool as the ``CSPSetting.identifier`` field. + + Depending on the analytic tool used, the ``CSPSetting.value`` field can be fixed + (e.g. with GA or Siteimprove) or configured by the user. The latter requires more work, + as we need to iterate over the ``AnalyticsToolsConfiguration`` fields to find the right match. + """ + + AnalyticsToolsConfiguration = apps.get_model( + "analytics_tools", "AnalyticsToolsConfiguration" + ) + try: + analytics_conf = AnalyticsToolsConfiguration.objects.get() + except AnalyticsToolsConfiguration.DoesNotExist: + return + + ContentType = apps.get_model("contenttypes", "ContentType") + analytics_content_type = ContentType.objects.get_for_model( + AnalyticsToolsConfiguration + ) + + CSPSetting = apps.get_model("config", "CSPSetting") + + set_content_type = False + for csp_setting in CSPSetting.objects.filter(identifier="").iterator(): + if csp_setting.value in SITEIMPROVE_VALUES: + csp_setting.identifier = AnalyticsTools.siteimprove + set_content_type = True + elif csp_setting.value in GA_VALUES: + csp_setting.identifier = AnalyticsTools.google_analytics + set_content_type = True + else: + for field, identifier in FIELD_TO_IDENTIFIER.items(): + if getattr(analytics_conf, field, None) == csp_setting.value: + csp_setting.identifier = identifier + set_content_type = True + + if set_content_type: + # `content_object` is not available in migrations, + # so we set `content_type` and `object_id` instead: + csp_setting.content_type = analytics_content_type + csp_setting.object_id = analytics_conf.id + + csp_setting.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("analytics_tools", "0002_auto_20230119_1500"), + ("config", "0063_auto_20231122_1816"), + ] + + operations = [ + migrations.RunPython(set_identifier, migrations.RunPython.noop), + ] diff --git a/src/openforms/analytics_tools/models.py b/src/openforms/analytics_tools/models.py index ae850221a3..6b6c6f37f7 100644 --- a/src/openforms/analytics_tools/models.py +++ b/src/openforms/analytics_tools/models.py @@ -240,6 +240,7 @@ def is_google_analytics_enabled(self) -> bool: def save(self, *args, **kwargs): # If instance is being created, we can't find original values + # (we use the _state API and not self.pk because `SingletonModel` hardcodes the PK). if self._state.adding: return super().save(*args, **kwargs) diff --git a/src/openforms/analytics_tools/tests/test_cspsettings_identifier.py b/src/openforms/analytics_tools/tests/test_cspsettings_identifier.py new file mode 100644 index 0000000000..2b1bbd8d45 --- /dev/null +++ b/src/openforms/analytics_tools/tests/test_cspsettings_identifier.py @@ -0,0 +1,44 @@ +from django.contrib.admin.options import get_content_type_for_model +from django.test import TestCase, override_settings + +from openforms.config.models import CSPSetting + +from ..constants import AnalyticsTools +from .mixin import AnalyticsMixin + + +@override_settings(SOLO_CACHE=None) +class CSPIdentifierTests(AnalyticsMixin, TestCase): + def test_disabling_analytics_does_not_delete_unrelated_csp_settings(self): + self.config.gtm_code = "GTM-XXXX" + self.config.ga_code = "UA-XXXXX-Y" + self.config.enable_google_analytics = True + + self.config.matomo_url = "https://example.com" + self.config.matomo_site_id = "1234" + self.config.enable_matomo_site_analytics = True + self.config.clean() + self.config.save() + + csp_settings = CSPSetting.objects.filter( + content_type=get_content_type_for_model(self.config), + object_id=str(self.config.pk), + ) + + # GA sets two CSPSettings + assert csp_settings.count() == 3 + + self.config.enable_google_analytics = False + self.config.save() + + try: + CSPSetting.objects.get(identifier=AnalyticsTools.matomo) + except CSPSetting.DoesNotExist: + self.fail("CSPSetting instance should exist") + + other_settings = CSPSetting.objects.filter( + content_type=get_content_type_for_model(self.config), + object_id=str(self.config.pk), + identifier=AnalyticsTools.google_analytics, + ) + self.assertFalse(other_settings.exists()) diff --git a/src/openforms/analytics_tools/tests/test_google_analytics.py b/src/openforms/analytics_tools/tests/test_google_analytics.py index 6b6c48013b..be713b11a4 100644 --- a/src/openforms/analytics_tools/tests/test_google_analytics.py +++ b/src/openforms/analytics_tools/tests/test_google_analytics.py @@ -5,6 +5,7 @@ from openforms.config.models import CSPSetting +from ..constants import AnalyticsTools from .mixin import AnalyticsMixin @@ -42,7 +43,9 @@ def test_google_analytics_properly_enabled(self): with self.subTest("Test creation of CSP"): try: CSPSetting.objects.get( - value=csp["value"], directive=csp["directive"] + value=csp["value"], + directive=csp["directive"], + identifier=AnalyticsTools.google_analytics, ) except CSPSetting.DoesNotExist as e: self.fail(f"Unexpected exception : {e}") @@ -69,7 +72,9 @@ def test_google_analytics_properly_disabled(self): with self.subTest("Test deletion of CSP"): self.assertFalse( CSPSetting.objects.filter( - value=csp["value"], directive=csp["directive"] + value=csp["value"], + directive=csp["directive"], + identifier=AnalyticsTools.google_analytics, ).exists() ) diff --git a/src/openforms/analytics_tools/tests/test_matomo.py b/src/openforms/analytics_tools/tests/test_matomo.py index 5d34e4961a..c2f4643609 100644 --- a/src/openforms/analytics_tools/tests/test_matomo.py +++ b/src/openforms/analytics_tools/tests/test_matomo.py @@ -5,6 +5,7 @@ from openforms.config.models import CSPSetting +from ..constants import AnalyticsTools from .mixin import AnalyticsMixin @@ -42,7 +43,9 @@ def test_matomo_properly_enabled(self): with self.subTest("Test creation of CSP"): try: CSPSetting.objects.get( - value=csp["value"], directive=csp["directive"] + value=csp["value"], + directive=csp["directive"], + identifier=AnalyticsTools.matomo, ) except CSPSetting.DoesNotExist as e: self.fail(f"Unexpected exception : {e}") @@ -69,7 +72,9 @@ def test_matomo_properly_disabled(self): with self.subTest("Test deletion of CSP"): self.assertFalse( CSPSetting.objects.filter( - value=csp["value"], directive=csp["directive"] + value=csp["value"], + directive=csp["directive"], + identifier=AnalyticsTools.matomo, ).exists() ) diff --git a/src/openforms/analytics_tools/tests/test_migrations.py b/src/openforms/analytics_tools/tests/test_migrations.py new file mode 100644 index 0000000000..2c9f74d3d8 --- /dev/null +++ b/src/openforms/analytics_tools/tests/test_migrations.py @@ -0,0 +1,77 @@ +from django.contrib.admin.options import get_content_type_for_model +from django.db.migrations.state import StateApps + +from openforms.analytics_tools.constants import AnalyticsTools +from openforms.config.constants import CSPDirective +from openforms.utils.tests.test_migrations import TestMigrations + + +class CSPSettingIdentifierMigrationTests(TestMigrations): + app = "analytics_tools" + migrate_from = "0002_auto_20230119_1500" + migrate_to = "0003_cspsetting_identifier" + + def setUpBeforeMigration(self, apps: StateApps): + CSPSetting = apps.get_model("config", "CSPSetting") + AnalyticsToolsConfiguration = apps.get_model( + "analytics_tools", "AnalyticsToolsConfiguration" + ) + + AnalyticsToolsConfiguration.objects.create( + matomo_url="https://matomo.example.com", + piwik_url="https://piwik.example.com", + piwik_pro_url="https://your-instance-name.piwik.pro", + ) + + CSPSetting.objects.create( + directive=CSPDirective.DEFAULT_SRC, value="https://matomo.example.com" + ) + CSPSetting.objects.create( + directive=CSPDirective.DEFAULT_SRC, value="https://piwik.example.com" + ) + CSPSetting.objects.create( + directive=CSPDirective.DEFAULT_SRC, + value="https://your-instance-name.piwik.pro", + ) + CSPSetting.objects.create( + directive=CSPDirective.DEFAULT_SRC, value="https://siteimproveanalytics.com" + ) + CSPSetting.objects.create( + directive=CSPDirective.DEFAULT_SRC, value="https://www.googleanalytics.com" + ) + + def test_migration_sets_identifier_and_gfk(self): + CSPSetting = self.apps.get_model("config", "CSPSetting") + AnalyticsToolsConfiguration = self.apps.get_model( + "analytics_tools", "AnalyticsToolsConfiguration" + ) + analytics_conf = AnalyticsToolsConfiguration.objects.get() + + value_to_identifier = { + "https://matomo.example.com": AnalyticsTools.matomo, + "https://piwik.example.com": AnalyticsTools.piwik, + "https://your-instance-name.piwik.pro": AnalyticsTools.piwik_pro, + "https://siteimproveanalytics.com": AnalyticsTools.siteimprove, + "https://www.googleanalytics.com": AnalyticsTools.google_analytics, + } + + self.assertFalse(CSPSetting.objects.filter(identifier="").exists()) + print(type(get_content_type_for_model(analytics_conf))) + + # We avoid using django.contrib.admin.options.get_content_type_for_model + # as it uses the "real" `ContentType` model. See: + # https://stackoverflow.com/q/51670468/#comment110467392_54357872 + content_type = self.apps.get_model( + "contenttypes", "ContentType" + ).objects.get_for_model(analytics_conf) + + for value, identifier in value_to_identifier.items(): + self.assertEqual( + CSPSetting.objects.filter( + value=value, + identifier=identifier, + content_type=content_type, + object_id=str(analytics_conf.pk), + ).count(), + 1, + ) diff --git a/src/openforms/analytics_tools/tests/test_piwik.py b/src/openforms/analytics_tools/tests/test_piwik.py index 2ec63cc36b..ed3610f7de 100644 --- a/src/openforms/analytics_tools/tests/test_piwik.py +++ b/src/openforms/analytics_tools/tests/test_piwik.py @@ -5,6 +5,7 @@ from openforms.config.models import CSPSetting +from ..constants import AnalyticsTools from .mixin import AnalyticsMixin @@ -40,7 +41,9 @@ def test_piwik_properly_enabled(self): with self.subTest("Test creation of CSP"): try: CSPSetting.objects.get( - value=csp["value"], directive=csp["directive"] + value=csp["value"], + directive=csp["directive"], + identifier=AnalyticsTools.piwik, ) except CSPSetting.DoesNotExist as e: self.fail(f"Unexpected exception : {e}") @@ -67,7 +70,9 @@ def test_piwik_properly_disabled(self): with self.subTest("Test deletion of CSP"): self.assertFalse( CSPSetting.objects.filter( - value=csp["value"], directive=csp["directive"] + value=csp["value"], + directive=csp["directive"], + identifier=AnalyticsTools.piwik, ).exists() ) diff --git a/src/openforms/analytics_tools/tests/test_piwik_pro.py b/src/openforms/analytics_tools/tests/test_piwik_pro.py index 06aec7b9fc..6f6d18560b 100644 --- a/src/openforms/analytics_tools/tests/test_piwik_pro.py +++ b/src/openforms/analytics_tools/tests/test_piwik_pro.py @@ -10,6 +10,7 @@ from openforms.forms.tests.factories import FormFactory from openforms.tests.test_csp import CSPMixin +from ..constants import AnalyticsTools from .mixin import AnalyticsMixin @@ -47,7 +48,9 @@ def test_piwik_pro_properly_enabled(self): with self.subTest("Test creation of CSP"): try: CSPSetting.objects.get( - value=csp["value"], directive=csp["directive"] + value=csp["value"], + directive=csp["directive"], + identifier=AnalyticsTools.piwik_pro, ) except CSPSetting.DoesNotExist as e: self.fail(f"Unexpected exception : {e}") @@ -74,7 +77,9 @@ def test_piwik_pro_properly_disabled(self): with self.subTest("Test deletion of CSP"): self.assertFalse( CSPSetting.objects.filter( - value=csp["value"], directive=csp["directive"] + value=csp["value"], + directive=csp["directive"], + identifier=AnalyticsTools.piwik_pro, ).exists() ) diff --git a/src/openforms/analytics_tools/tests/test_site_improve.py b/src/openforms/analytics_tools/tests/test_site_improve.py index a5d18a2bf8..15b36eadb6 100644 --- a/src/openforms/analytics_tools/tests/test_site_improve.py +++ b/src/openforms/analytics_tools/tests/test_site_improve.py @@ -5,6 +5,7 @@ from openforms.config.models import CSPSetting +from ..constants import AnalyticsTools from .mixin import AnalyticsMixin @@ -42,7 +43,9 @@ def test_site_improve_properly_enabled(self): with self.subTest("Test creation of CSP"): try: CSPSetting.objects.get( - value=csp["value"], directive=csp["directive"] + value=csp["value"], + directive=csp["directive"], + identifier=AnalyticsTools.siteimprove, ) except CSPSetting.DoesNotExist as e: self.fail(f"Unexpected exception : {e}") @@ -68,7 +71,9 @@ def test_site_improve_properly_disabled(self): with self.subTest("Test deletion of CSP"): self.assertFalse( CSPSetting.objects.filter( - value=csp["value"], directive=csp["directive"] + value=csp["value"], + directive=csp["directive"], + identifier=AnalyticsTools.siteimprove, ).exists() ) diff --git a/src/openforms/analytics_tools/utils.py b/src/openforms/analytics_tools/utils.py index dad4fd010f..6ab0cea22e 100644 --- a/src/openforms/analytics_tools/utils.py +++ b/src/openforms/analytics_tools/utils.py @@ -8,9 +8,12 @@ from cookie_consent.models import Cookie, CookieGroup +from openforms.config.constants import CSPDirective from openforms.config.models import CSPSetting from openforms.typing import JSONObject +from .constants import AnalyticsTools + if TYPE_CHECKING: # pragma: no cover from .models import AnalyticsToolsConfiguration, ToolConfiguration @@ -24,16 +27,16 @@ class CookieDict(TypedDict): class CSPDict(TypedDict): - directive: str + directive: CSPDirective value: str def update_analytics_tool( config: "AnalyticsToolsConfiguration", - analytics_tool: str, + analytics_tool: AnalyticsTools, is_activated: bool, tool_config: "ToolConfiguration", -): +) -> None: from openforms.logging import logevent if is_activated: @@ -42,7 +45,12 @@ def update_analytics_tool( logevent.disabling_analytics_tool(config, analytics_tool) # process the CSP headers - csps = cast(list[CSPDict], load_asset("csp_headers.json", analytics_tool)) + csps = ( + [] + if not is_activated + else cast(list[CSPDict], load_asset("csp_headers.json", analytics_tool)) + ) + for csp in csps: for replacement in tool_config.replacements: if not (field_name := replacement.field_name): @@ -52,6 +60,12 @@ def update_analytics_tool( replacement.needle, str(replacement_value) ) + CSPSetting.objects.set_for( + config, + [(csp["directive"], csp["value"]) for csp in csps], + identifier=analytics_tool, + ) + # process the cookies cookies = cast(list[CookieDict], load_asset("cookies.json", analytics_tool)) for cookie in cookies: @@ -69,7 +83,6 @@ def update_analytics_tool( create=is_activated, cookie_consent_group_id=config.analytics_cookie_consent_group.id, ) - update_csp(csps, create=is_activated) def load_asset( @@ -128,13 +141,3 @@ def update_analytical_cookies( Cookie.objects.bulk_create(instances) else: Cookie.objects.filter(name__in=[cookie["name"] for cookie in cookies]).delete() - - -def update_csp(csps: list[CSPDict], create: bool): - if create: - instances = [ - CSPSetting(directive=csp["directive"], value=csp["value"]) for csp in csps - ] - CSPSetting.objects.bulk_create(instances) - else: - CSPSetting.objects.filter(value__in=[csp["value"] for csp in csps]).delete() diff --git a/src/openforms/config/admin.py b/src/openforms/config/admin.py index ca461601c1..81c0b24708 100644 --- a/src/openforms/config/admin.py +++ b/src/openforms/config/admin.py @@ -188,18 +188,22 @@ class CSPSettingAdmin(admin.ModelAdmin): fields = [ "directive", "value", + "identifier", "content_type_link", ] list_display = [ "directive", "value", + "identifier", ] list_filter = [ "directive", + "identifier", ] search_fields = [ "directive", "value", + "identifier", ] def content_type_link(self, obj): diff --git a/src/openforms/config/migrations/0062_cspsetting_identifier.py b/src/openforms/config/migrations/0062_cspsetting_identifier.py new file mode 100644 index 0000000000..08c3f74368 --- /dev/null +++ b/src/openforms/config/migrations/0062_cspsetting_identifier.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.21 on 2023-11-17 15:10 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("config", "0061_convert_container_layout_design_tokens"), + ] + + operations = [ + migrations.AddField( + model_name="cspsetting", + name="identifier", + field=models.CharField( + blank=True, + help_text="An extra tag for this CSP entry, to identify the exact source", + max_length=64, + verbose_name="identifier", + ), + ), + ] diff --git a/src/openforms/config/migrations/0063_auto_20231122_1816.py b/src/openforms/config/migrations/0063_auto_20231122_1816.py new file mode 100644 index 0000000000..406130a32a --- /dev/null +++ b/src/openforms/config/migrations/0063_auto_20231122_1816.py @@ -0,0 +1,67 @@ +# Generated by Django 3.2.21 on 2023-11-22 17:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("config", "0062_cspsetting_identifier"), + ] + + operations = [ + migrations.AlterField( + model_name="cspsetting", + name="directive", + field=models.CharField( + choices=[ + ("default-src", "default-src"), + ("script-src", "script-src"), + ("script-src-attr", "script-src-attr"), + ("script-src-elem", "script-src-elem"), + ("img-src", "img-src"), + ("object-src", "object-src"), + ("prefetch-src", "prefetch-src"), + ("media-src", "media-src"), + ("frame-src", "frame-src"), + ("font-src", "font-src"), + ("connect-src", "connect-src"), + ("style-src", "style-src"), + ("style-src-attr", "style-src-attr"), + ("style-src-elem", "style-src-elem"), + ("base-uri", "base-uri"), + ("child-src", "child-src"), + ("frame-ancestors", "frame-ancestors"), + ("navigate-to", "navigate-to"), + ("form-action", "form-action"), + ("sandbox", "sandbox"), + ("report-uri", "report-uri"), + ("report-to", "report-to"), + ("manifest-src", "manifest-src"), + ("worker-src", "worker-src"), + ("plugin-types", "plugin-types"), + ("require-sri-for", "require-sri-for"), + ], + help_text="CSP header directive.", + max_length=64, + verbose_name="directive", + ), + ), + migrations.AlterField( + model_name="cspsetting", + name="identifier", + field=models.CharField( + blank=True, + help_text="An extra tag for this CSP entry, to identify the exact source.", + max_length=64, + verbose_name="identifier", + ), + ), + 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 ea6e4e3a4e..bb8e5fe871 100644 --- a/src/openforms/config/models.py +++ b/src/openforms/config/models.py @@ -661,18 +661,32 @@ def as_dict(self): class CSPSettingManager(models.Manager.from_queryset(CSPSettingQuerySet)): @transaction.atomic def set_for( - self, obj: models.Model, settings: list[tuple[CSPDirective, str]] + self, + obj: models.Model, + settings: list[tuple[CSPDirective, str]], + identifier: str = "", ) -> None: """ - Deletes all the connected csp settings and creates new ones based on the new provided data. + Deletes all the connected CSP settings and creates new ones based on the new provided data. + + :param obj: The configuration model providing this CSP entry. + :param settings: A two-tuple containing values used to create the underlying ``CSPSetting`` model. + :param identifier: An optional string to further identify the source of this CSP entry. """ instances = [ - CSPSetting(content_object=obj, directive=directive, value=value) + CSPSetting( + content_object=obj, + directive=directive, + value=value, + identifier=identifier, + ) for directive, value in settings ] CSPSetting.objects.filter( - content_type=get_content_type_for_model(obj), object_id=str(obj.id) + content_type=get_content_type_for_model(obj), + object_id=str(obj.id), + identifier=identifier, ).delete() self.bulk_create(instances) @@ -682,14 +696,23 @@ class CSPSetting(models.Model): directive = models.CharField( _("directive"), max_length=64, - help_text=_("CSP header directive"), choices=CSPDirective.choices, + help_text=_("CSP header directive."), ) value = models.CharField( _("value"), max_length=255, - help_text=_("CSP header value"), + help_text=_("CSP header value."), ) + + identifier = models.CharField( + _("identifier"), + max_length=64, + blank=True, + help_text=_("An extra tag for this CSP entry, to identify the exact source."), + ) + + # Generic relation fields (see https://docs.djangoproject.com/en/dev/ref/contrib/contenttypes/#generic-relations): content_type = models.ForeignKey( ContentType, verbose_name=_("content type"), @@ -704,7 +727,7 @@ class CSPSetting(models.Model): ) content_object = GenericForeignKey("content_type", "object_id") - objects = CSPSettingManager() + objects: CSPSettingManager = CSPSettingManager() class Meta: ordering = ("directive", "value")