From 912edb5b91b486307e00b109325481ecfa8a6d7b Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Tue, 3 Sep 2024 10:23:04 +0200 Subject: [PATCH 1/4] [#2723] Support multiple backends for Notifications API --- src/notifications/__init__.py | 0 src/notifications/admin.py | 37 +++++ src/notifications/api/__init__.py | 0 src/notifications/api/serializers.py | 59 ++++++++ src/notifications/apps.py | 8 + src/notifications/migrations/0001_initial.py | 131 +++++++++++++++++ src/notifications/migrations/__init__.py | 0 src/notifications/models.py | 137 ++++++++++++++++++ src/notifications/query.py | 7 + src/notifications/tests/__init__.py | 0 src/notifications/tests/test_webhook.py | 109 ++++++++++++++ src/notifications/tests/utils.py | 16 ++ src/notifications/validators.py | 35 +++++ src/open_inwoner/conf/base.py | 1 + .../conf/fixtures/django-admin-index.json | 4 +- src/open_inwoner/openzaak/api/views.py | 6 +- src/open_inwoner/openzaak/auth.py | 2 +- src/open_inwoner/openzaak/tests/factories.py | 12 +- .../tests/test_notification_zaak_status.py | 2 + 19 files changed, 559 insertions(+), 7 deletions(-) create mode 100644 src/notifications/__init__.py create mode 100644 src/notifications/admin.py create mode 100644 src/notifications/api/__init__.py create mode 100644 src/notifications/api/serializers.py create mode 100644 src/notifications/apps.py create mode 100644 src/notifications/migrations/0001_initial.py create mode 100644 src/notifications/migrations/__init__.py create mode 100644 src/notifications/models.py create mode 100644 src/notifications/query.py create mode 100644 src/notifications/tests/__init__.py create mode 100644 src/notifications/tests/test_webhook.py create mode 100644 src/notifications/tests/utils.py create mode 100644 src/notifications/validators.py diff --git a/src/notifications/__init__.py b/src/notifications/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/notifications/admin.py b/src/notifications/admin.py new file mode 100644 index 0000000000..e95801c396 --- /dev/null +++ b/src/notifications/admin.py @@ -0,0 +1,37 @@ +from django.contrib import admin, messages +from django.utils.translation import gettext_lazy as _ + +from requests.exceptions import RequestException + +from .models import NotificationsAPIConfig, Subscription + + +@admin.register(NotificationsAPIConfig) +class NotificationsConfigAdmin(admin.ModelAdmin): + pass + + +def register_webhook(modeladmin, request, queryset): + for sub in queryset: + if sub._subscription: + continue + + try: + sub.register() + except RequestException as exc: + messages.error( + request, + _( + "Something went wrong while registering subscription " + "for {callback}: {exception}" + ).format(callback=sub.callback_url, exception=exc), + ) + + +register_webhook.short_description = _("Register the webhooks") # noqa + + +@admin.register(Subscription) +class SubscriptionAdmin(admin.ModelAdmin): + list_display = ("callback_url", "channels", "_subscription") + actions = [register_webhook] diff --git a/src/notifications/api/__init__.py b/src/notifications/api/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/notifications/api/serializers.py b/src/notifications/api/serializers.py new file mode 100644 index 0000000000..a90ef34c17 --- /dev/null +++ b/src/notifications/api/serializers.py @@ -0,0 +1,59 @@ +from django.utils.translation import gettext_lazy as _ + +from rest_framework import serializers + +from ..validators import UntilNowValidator + + +class NotificatieSerializer(serializers.Serializer): + kanaal = serializers.CharField( + label=_("kanaal"), + max_length=50, + help_text=_( + "De naam van het kanaal (`KANAAL.naam`) waar het bericht " + "op moet worden gepubliceerd." + ), + ) + hoofd_object = serializers.URLField( + label=_("hoofd object"), + help_text=_( + "URL-referentie naar het hoofd object van de publicerende " + "API die betrekking heeft op de `resource`." + ), + ) + resource = serializers.CharField( + label=_("resource"), + max_length=100, + help_text=_("De resourcenaam waar de notificatie over gaat."), + ) + resource_url = serializers.URLField( + label=_("resource URL"), + help_text=_("URL-referentie naar de `resource` van de publicerende " "API."), + ) + actie = serializers.CharField( + label=_("actie"), + max_length=100, + help_text=_( + "De actie die door de publicerende API is gedaan. De " + "publicerende API specificeert de toegestane acties." + ), + ) + aanmaakdatum = serializers.DateTimeField( + label=_("aanmaakdatum"), + validators=[UntilNowValidator()], + help_text=_("Datum en tijd waarop de actie heeft plaatsgevonden."), + ) + kenmerken = serializers.DictField( + label=_("kenmerken"), + required=False, + child=serializers.CharField( + label=_("kenmerk"), + max_length=1000, + help_text=_("Een waarde behorende bij de sleutel."), + allow_blank=True, + ), + help_text=_( + "Mapping van kenmerken (sleutel/waarde) van de notificatie. De " + "publicerende API specificeert de toegestane kenmerken." + ), + ) diff --git a/src/notifications/apps.py b/src/notifications/apps.py new file mode 100644 index 0000000000..719d672d8c --- /dev/null +++ b/src/notifications/apps.py @@ -0,0 +1,8 @@ +from django.apps import AppConfig +from django.utils.translation import gettext_lazy as _ + + +class NotificationsConfig(AppConfig): + name = "notifications" + verbose_name = _("Notifications API integration") + default_auto_field = "django.db.models.BigAutoField" diff --git a/src/notifications/migrations/0001_initial.py b/src/notifications/migrations/0001_initial.py new file mode 100644 index 0000000000..ae155a5693 --- /dev/null +++ b/src/notifications/migrations/0001_initial.py @@ -0,0 +1,131 @@ +# Generated by Django 4.2.15 on 2024-09-04 10:18 + +import django.contrib.postgres.fields +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ("zgw_consumers", "0022_set_default_service_slug"), + ] + + operations = [ + migrations.CreateModel( + name="NotificationsAPIConfig", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "notification_delivery_max_retries", + models.PositiveIntegerField( + default=5, + help_text="The maximum number of automatic retries. After this amount of retries, guaranteed delivery stops trying to deliver the message.", + ), + ), + ( + "notification_delivery_retry_backoff", + models.PositiveIntegerField( + default=3, + help_text="If specified, a factor applied to the exponential backoff. This allows you to tune how quickly automatic retries are performed.", + ), + ), + ( + "notification_delivery_retry_backoff_max", + models.PositiveIntegerField( + default=48, + help_text="An upper limit in seconds to the exponential backoff time.", + ), + ), + ( + "notifications_api_service", + models.ForeignKey( + limit_choices_to={"api_type": "nrc"}, + on_delete=django.db.models.deletion.PROTECT, + related_name="notifications_api_configs", + to="zgw_consumers.service", + verbose_name="notifications api service", + ), + ), + ], + options={ + "verbose_name": "Notificatiescomponentenconfiguratie", + }, + ), + migrations.CreateModel( + name="Subscription", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "callback_url", + models.URLField( + help_text="Where to send the notifications (webhook url)", + verbose_name="callback url", + ), + ), + ( + "client_id", + models.CharField( + help_text="Client ID to construct the auth token", + max_length=50, + verbose_name="client ID", + ), + ), + ( + "secret", + models.CharField( + help_text="Secret to construct the auth token", + max_length=50, + verbose_name="client secret", + ), + ), + ( + "channels", + django.contrib.postgres.fields.ArrayField( + base_field=models.CharField(max_length=100), + help_text="Comma-separated list of channels to subscribe to", + size=None, + verbose_name="channels", + ), + ), + ( + "_subscription", + models.URLField( + blank=True, + editable=False, + help_text="Subscription as it is known in the NC", + verbose_name="NC subscription", + ), + ), + ( + "notifications_api_config", + models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + to="notifications.notificationsapiconfig", + ), + ), + ], + options={ + "verbose_name": "Webhook subscription", + "verbose_name_plural": "Webhook subscriptions", + }, + ), + ] diff --git a/src/notifications/migrations/__init__.py b/src/notifications/migrations/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/notifications/models.py b/src/notifications/models.py new file mode 100644 index 0000000000..5d8730cccd --- /dev/null +++ b/src/notifications/models.py @@ -0,0 +1,137 @@ +from django.contrib.postgres.fields import ArrayField +from django.db import models +from django.utils.translation import gettext_lazy as _ + +from ape_pie import APIClient +from zds_client import ClientAuth +from zgw_consumers.client import build_client +from zgw_consumers.constants import APITypes +from zgw_consumers.models import Service + +from .query import NotificationsConfigManager + + +class NotificationsAPIConfig(models.Model): + notifications_api_service = models.ForeignKey( + to=Service, + related_name="notifications_api_configs", + limit_choices_to={"api_type": APITypes.nrc}, + verbose_name=_("notifications api service"), + on_delete=models.PROTECT, + ) + notification_delivery_max_retries = models.PositiveIntegerField( + help_text=_( + "The maximum number of automatic retries. After this amount of retries, " + "guaranteed delivery stops trying to deliver the message." + ), + default=5, + ) + notification_delivery_retry_backoff = models.PositiveIntegerField( + help_text=_( + "If specified, a factor applied to the exponential backoff. " + "This allows you to tune how quickly automatic retries are performed." + ), + default=3, + ) + notification_delivery_retry_backoff_max = models.PositiveIntegerField( + help_text=_("An upper limit in seconds to the exponential backoff time."), + default=48, + ) + + objects = NotificationsConfigManager() + + class Meta: + verbose_name = _("Notificatiescomponentenconfiguratie") + + def __str__(self): + api_root = ( + self.notifications_api_service.api_root + if self.notifications_api_service + else _("no service configured") + ) + return _("Notifications API configuration ({api_root})").format( + api_root=api_root + ) + + def get_client(self) -> APIClient | None: + """ + Construct a client, prepared with the required auth. + """ + if self.notifications_api_service: + return build_client(self.notifications_api_service) + return None + + +class Subscription(models.Model): + notifications_api_config = models.ForeignKey( + to=NotificationsAPIConfig, + on_delete=models.PROTECT, + ) + callback_url = models.URLField( + _("callback url"), help_text=_("Where to send the notifications (webhook url)") + ) + client_id = models.CharField( + _("client ID"), + max_length=50, + help_text=_("Client ID to construct the auth token"), + ) + secret = models.CharField( + _("client secret"), + max_length=50, + help_text=_("Secret to construct the auth token"), + ) + channels = ArrayField( + models.CharField(max_length=100), + verbose_name=_("channels"), + help_text=_("Comma-separated list of channels to subscribe to"), + ) + + _subscription = models.URLField( + _("NC subscription"), + blank=True, + editable=False, + help_text=_("Subscription as it is known in the NC"), + ) + + class Meta: + verbose_name = _("Webhook subscription") + verbose_name_plural = _("Webhook subscriptions") + + def __str__(self): + return f"{', '.join(self.channels)} - {self.callback_url}" + + def register(self) -> None: + """ + Registers the webhook with the notification component. + """ + assert ( + self.notifications_api_config.notifications_api_service + ), "No service for Notifications API configured" + + client = self.notifications_api_config.get_client() + + # This authentication is for the NC to call us. Thus, it's *not* for + # calling the NC to create a subscription. + # TODO replace with `TokenAuth`? + # see: https://github.com/maykinmedia/notifications-api-common/pull/1#discussion_r941450384 + self_auth = ClientAuth( + client_id=self.client_id, + secret=self.secret, + ) + data = { + "callbackUrl": self.callback_url, + "auth": self_auth.credentials()["Authorization"], + "kanalen": [ + { + "naam": channel, + "filters": {}, + } + for channel in self.channels + ], + } + + response = client.post("abonnement", json=data) + response_json = response.json() + + self._subscription = response_json["url"] + self.save(update_fields=["_subscription"]) diff --git a/src/notifications/query.py b/src/notifications/query.py new file mode 100644 index 0000000000..e76036ef7c --- /dev/null +++ b/src/notifications/query.py @@ -0,0 +1,7 @@ +from django.db import models + + +class NotificationsConfigManager(models.Manager): + def get_queryset(self): + qs = super().get_queryset() + return qs.select_related("notifications_api_service") diff --git a/src/notifications/tests/__init__.py b/src/notifications/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/notifications/tests/test_webhook.py b/src/notifications/tests/test_webhook.py new file mode 100644 index 0000000000..1707ba8f03 --- /dev/null +++ b/src/notifications/tests/test_webhook.py @@ -0,0 +1,109 @@ +from unittest.mock import patch + +from django.contrib.messages import get_messages +from django.test import TestCase + +import requests_mock +from requests.exceptions import RequestException +from zgw_consumers.constants import APITypes +from zgw_consumers.models.services import Service + +from ..admin import register_webhook +from ..models import NotificationsAPIConfig, Subscription +from .utils import make_request_with_middleware + + +class NotificationsAPIWebhookTest(TestCase): + @classmethod + def setUpTestData(cls): + service = Service.objects.create( + api_root="http://some-api-root/api/v1/", + api_type=APITypes.nrc, + slug="service", + oas="http://some-api-root/api/v1/schema/openapi.yaml", + ) + cls.config = NotificationsAPIConfig.objects.create( + notifications_api_service=service + ) + cls.config.save() + + service_other = Service.objects.create( + api_root="http://other-api-root/api/v1/", + api_type=APITypes.nrc, + slug="service-other", + oas="http://other-api-root/api/v1/schema/openapi.yaml", + ) + cls.config_other = NotificationsAPIConfig.objects.create( + notifications_api_service=service_other + ) + cls.config_other.save() + + @requests_mock.Mocker() + def test_register_webhook_success(self, m): + m.post( + "http://some-api-root/api/v1/abonnement", + json={ + "url": "https://example.com/api/v1/abonnementen/1", + }, + ) + m.post( + "http://other-api-root/api/v1/abonnement", + json={ + "url": "https://example.com/api/v1/abonnementen/2", + }, + ) + + subscription = Subscription.objects.create( + notifications_api_config=self.config, + callback_url="https://example.com/callback", + client_id="client_id", + secret="secret", + channels=["zaken"], + ) + subscription_other = Subscription.objects.create( + notifications_api_config=self.config_other, + callback_url="https://example.com/callback", + client_id="client_id", + secret="secret", + channels=["zaken"], + ) + + request_with_middleware = make_request_with_middleware() + + register_webhook(object, request_with_middleware, Subscription.objects.all()) + + messages = list(get_messages(request_with_middleware)) + + self.assertEqual(len(messages), 0) + + subscription.refresh_from_db() + self.assertEqual( + subscription._subscription, "https://example.com/api/v1/abonnementen/1" + ) + subscription_other.refresh_from_db() + self.assertEqual( + subscription_other._subscription, + "https://example.com/api/v1/abonnementen/2", + ) + + def test_register_webhook_request_exception(self): + Subscription.objects.create( + notifications_api_config=self.config, + callback_url="https://example.com/callback", + client_id="client_id", + secret="secret", + channels=["zaken"], + ) + + request_with_middleware = make_request_with_middleware() + + with patch( + "requests.sessions.Session.post", side_effect=RequestException("exception") + ): + register_webhook( + object, request_with_middleware, Subscription.objects.all() + ) + + messages = list(get_messages(request_with_middleware)) + + self.assertEqual(len(messages), 1) diff --git a/src/notifications/tests/utils.py b/src/notifications/tests/utils.py new file mode 100644 index 0000000000..a57353a603 --- /dev/null +++ b/src/notifications/tests/utils.py @@ -0,0 +1,16 @@ +from django.contrib.messages.middleware import MessageMiddleware +from django.contrib.sessions.middleware import SessionMiddleware +from django.test import RequestFactory + + +def dummy_get_response(request): + raise NotImplementedError() + + +def make_request_with_middleware(): + rf = RequestFactory() + + request = rf.get("/") + SessionMiddleware(get_response=dummy_get_response).process_request(request) + MessageMiddleware(get_response=dummy_get_response).process_request(request) + return request diff --git a/src/notifications/validators.py b/src/notifications/validators.py new file mode 100644 index 0000000000..0dfb48b9a0 --- /dev/null +++ b/src/notifications/validators.py @@ -0,0 +1,35 @@ +import logging + +from django.core.exceptions import ValidationError +from django.utils import timezone +from django.utils.deconstruct import deconstructible +from django.utils.translation import gettext_lazy as _ + +logger = logging.getLogger(__name__) + + +@deconstructible +class UntilNowValidator: + """ + Validate a datetime to not be in the future. + + This means that `now` is included. + """ + + message = _("Ensure this value is not in the future.") + code = "future_not_allowed" + + @property + def limit_value(self): + return timezone.now() + + def __call__(self, value): + if value > self.limit_value: + raise ValidationError(self.message, code=self.code) + + def __eq__(self, other): + return ( + isinstance(other, self.__class__) + and self.message == other.message + and self.code == other.code + ) diff --git a/src/open_inwoner/conf/base.py b/src/open_inwoner/conf/base.py index dfdebb8a0a..504f49f3ff 100644 --- a/src/open_inwoner/conf/base.py +++ b/src/open_inwoner/conf/base.py @@ -203,6 +203,7 @@ "formtools", "django_setup_configuration", "django_yubin", + "notifications", # Project applications. "open_inwoner.components", "open_inwoner.kvk", diff --git a/src/open_inwoner/conf/fixtures/django-admin-index.json b/src/open_inwoner/conf/fixtures/django-admin-index.json index cd5440bd33..fc3bde26b9 100644 --- a/src/open_inwoner/conf/fixtures/django-admin-index.json +++ b/src/open_inwoner/conf/fixtures/django-admin-index.json @@ -282,7 +282,9 @@ "model": "admin_index.appgroup", "fields": { "order": 5, - "translations": {}, + "translations": { + "nl": "Data koppelingen" + }, "name": "Data koppelingen", "slug": "koppelingen", "models": [ diff --git a/src/open_inwoner/openzaak/api/views.py b/src/open_inwoner/openzaak/api/views.py index c247a826ba..c87fdf8393 100644 --- a/src/open_inwoner/openzaak/api/views.py +++ b/src/open_inwoner/openzaak/api/views.py @@ -1,11 +1,11 @@ import logging -from notifications_api_common.api.serializers import NotificatieSerializer from rest_framework import status from rest_framework.response import Response from rest_framework.views import APIView from zgw_consumers.api_models.base import factory +from notifications.api.serializers import NotificatieSerializer from open_inwoner.configurations.models import SiteConfiguration from open_inwoner.openzaak.api_models import Notification from open_inwoner.openzaak.auth import get_valid_subscription_from_request @@ -18,9 +18,7 @@ class NotificationsWebhookBaseView(APIView): """ - generic ZGW notification webhook handler - - theoretically this could be moved to notifications-api-common + Generic ZGW notification webhook handler """ # optionally filter incoming channels diff --git a/src/open_inwoner/openzaak/auth.py b/src/open_inwoner/openzaak/auth.py index f8450dfb9b..b7b95b8e6e 100644 --- a/src/open_inwoner/openzaak/auth.py +++ b/src/open_inwoner/openzaak/auth.py @@ -2,9 +2,9 @@ import jwt from jwt import InvalidTokenError -from notifications_api_common.models import Subscription from zds_client.auth import JWT_ALG +from notifications.models import Subscription from open_inwoner.openzaak.exceptions import ( InvalidAuth, InvalidAuthForClientID, diff --git a/src/open_inwoner/openzaak/tests/factories.py b/src/open_inwoner/openzaak/tests/factories.py index 71bcd635d2..f52730ac05 100644 --- a/src/open_inwoner/openzaak/tests/factories.py +++ b/src/open_inwoner/openzaak/tests/factories.py @@ -2,7 +2,6 @@ import factory import faker -from notifications_api_common.models import Subscription from simple_certmanager.constants import CertificateTypes from simple_certmanager.models import Certificate from zgw_consumers.api_models.base import factory as zwg_factory @@ -11,6 +10,7 @@ from zgw_consumers.models import Service from zgw_consumers.test import generate_oas_component +from notifications.models import NotificationsAPIConfig, Subscription from open_inwoner.accounts.tests.factories import UserFactory from open_inwoner.openzaak.api_models import Notification, Rol from open_inwoner.openzaak.models import ( @@ -65,7 +65,17 @@ class Params: ) +class NotificationsAPIConfigFactory(factory.django.DjangoModelFactory): + notifications_api_service = factory.SubFactory( + ServiceFactory, api_type=APITypes.nrc + ) + + class Meta: + model = NotificationsAPIConfig + + class SubscriptionFactory(factory.django.DjangoModelFactory): + notifications_api_config = factory.SubFactory(NotificationsAPIConfigFactory) callback_url = factory.Faker("url") client_id = factory.Faker("word") secret = factory.Faker("pystr") diff --git a/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py b/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py index 1b29005d22..d4e80f8df0 100644 --- a/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py +++ b/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py @@ -49,6 +49,8 @@ def test_handle_zaak_status_notifications(self, m, mock_handle: Mock): data = MockAPIData().install_mocks(m) data_alt = MockAPIDataAlt().install_mocks(m) + self.clearTimelineLogs() + # Added for https://taiga.maykinmedia.nl/project/open-inwoner/task/1904 # In eSuite it is possible to reuse a StatusType for multiple ZaakTypen, which # led to errors when retrieving the ZaakTypeStatusTypeConfig. This duplicate From c74a892f8a3aea462379ff056863f41fb8a96609 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Fri, 13 Sep 2024 15:46:04 +0200 Subject: [PATCH 2/4] [#2723] Migrate data for integration of Notifications API from library --- ...rate_data_from_notifications_api_common.py | 126 ++++++++++++++++++ src/notifications/tests/test_migrations.py | 51 +++++++ 2 files changed, 177 insertions(+) create mode 100644 src/notifications/migrations/0002_migrate_data_from_notifications_api_common.py create mode 100644 src/notifications/tests/test_migrations.py diff --git a/src/notifications/migrations/0002_migrate_data_from_notifications_api_common.py b/src/notifications/migrations/0002_migrate_data_from_notifications_api_common.py new file mode 100644 index 0000000000..9b60681b75 --- /dev/null +++ b/src/notifications/migrations/0002_migrate_data_from_notifications_api_common.py @@ -0,0 +1,126 @@ +import logging + +from django.db import migrations + +logger = logging.getLogger(__name__) + + +def migrate_from_notifications_api_common(apps, schema_editor): + """ + Migrate data for integration of Notificaties API from library to OIP + """ + try: + LegacyNotificationsConfig = apps.get_model( + "notifications_api_common", "NotificationsConfig" + ) + LegacySubscription = apps.get_model("notifications_api_common", "Subscription") + except LookupError as exc: + exc.add_note( + "notifications_api_common must be in INSTALLED_APPS in order to run the migrations" + ) + raise + + NotificationsAPIConfig = apps.get_model("notifications", "NotificationsAPIConfig") + Subscription = apps.get_model("notifications", "Subscription") + + # migrate notifications config + try: + legacy_config = LegacyNotificationsConfig.objects.get() + except LegacyNotificationsConfig.DoesNotExist: + return + + if not legacy_config.notifications_api_service: + return + + config = NotificationsAPIConfig.objects.create( + notifications_api_service=getattr( + legacy_config, "notifications_api_service", None + ), + notification_delivery_max_retries=legacy_config.notification_delivery_max_retries, + notification_delivery_retry_backoff=legacy_config.notification_delivery_retry_backoff, + notification_delivery_retry_backoff_max=legacy_config.notification_delivery_retry_backoff_max, + ) + + # migrate subscriptions + legacy_subs = LegacySubscription.objects.all() + for legacy_sub in legacy_subs: + Subscription.objects.create( + notifications_api_config=config, + callback_url=legacy_sub.callback_url, + client_id=legacy_sub.client_id, + secret=legacy_sub.secret, + channels=legacy_sub.channels, + _subscription=legacy_sub._subscription, + ) + + +def reverse_migrate_from_notifications_api_common(apps, schema_editor): + """ + Reverse-migrate data for integration of Notificaties API to library + """ + try: + LegacyNotificationsConfig = apps.get_model( + "notifications_api_common", "NotificationsConfig" + ) + LegacySubscription = apps.get_model("notifications_api_common", "Subscription") + except LookupError as exc: + exc.add_note( + "notifications_api_common must be in INSTALLED_APPS in order to run the migrations" + ) + raise + + NotificationsAPIConfig = apps.get_model("notifications", "NotificationsAPIConfig") + Subscription = apps.get_model("notifications", "Subscription") + + # reverse-migrate config(s) + configs = NotificationsAPIConfig.objects.all() + + if configs.count() == 0: + logger.info( + "No configuration models for Notifications API found; " + "skipping data migration for notifications_api_common" + ) + return + elif configs.count() > 1: + raise ValueError( + "Multiple configuration models for Notifications API found; " + "reversing the migration for notifications_api_common requires that " + "there be only one configuration model" + ) + else: + config = configs.get() + LegacyNotificationsConfig.objects.create( + notifications_api_service=getattr( + config, "notifications_api_service", None + ), + notification_delivery_max_retries=config.notification_delivery_max_retries, + notification_delivery_retry_backoff=config.notification_delivery_retry_backoff, + notification_delivery_retry_backoff_max=config.notification_delivery_retry_backoff_max, + ) + + # reverse-migrate subscriptions + subs = Subscription.objects.all() + for sub in subs: + LegacySubscription.objects.create( + callback_url=sub.callback_url, + client_id=sub.client_id, + secret=sub.secret, + channels=sub.channels, + _subscription=sub._subscription, + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("notifications", "0001_initial"), + ( + "notifications_api_common", + "0008_merge_0006_auto_20221213_0214_0007_auto_20221206_0414", + ), + ] + operations = [ + migrations.RunPython( + code=migrate_from_notifications_api_common, + reverse_code=reverse_migrate_from_notifications_api_common, + ) + ] diff --git a/src/notifications/tests/test_migrations.py b/src/notifications/tests/test_migrations.py new file mode 100644 index 0000000000..c2f0253cc0 --- /dev/null +++ b/src/notifications/tests/test_migrations.py @@ -0,0 +1,51 @@ +from zgw_consumers.constants import APITypes + +from open_inwoner.openzaak.tests.factories import ServiceFactory +from open_inwoner.utils.tests.test_migrations import TestSuccessfulMigrations + + +class NotificationsAPILibraryMigrationTest(TestSuccessfulMigrations): + migrate_from = "0001_initial" + migrate_to = "0002_migrate_data_from_notifications_api_common" + app = "notifications" + + def setUpBeforeMigration(self, apps): + from notifications_api_common.models import ( + NotificationsConfig as LegacyNotificationsConfig, + Subscription as LegacySubscription, + ) + + self.notification_service = ServiceFactory(api_type=APITypes.nrc) + + legacy_notifications_config = LegacyNotificationsConfig.objects.get() + legacy_notifications_config.notifications_api_service = ( + self.notification_service + ) + legacy_notifications_config.save() + + LegacySubscription.objects.create( + callback_url="http://www.callback1.nl", + client_id="client_id_1", + secret="secret", + channels=["zaken"], + _subscription="http://subscription.nl", + ) + LegacySubscription.objects.create( + callback_url="http://www.callback2.nl", + client_id="client_id_2", + secret="secret_2", + channels=["zaken", "other"], + _subscription="http://subscription.nl", + ) + + def test_notifications_migration_0001_to_0002(self): + NotificationsAPIConfig = self.apps.get_model( + "notifications", "NotificationsAPIConfig" + ) + Subscription = self.apps.get_model("notifications", "Subscription") + + self.assertEqual(Subscription.objects.count(), 2) + self.assertEqual( + NotificationsAPIConfig.objects.get().notifications_api_service.uuid, + self.notification_service.uuid, + ) From 025b761e00845d499af055d183ca24b7f39f1bc3 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Fri, 13 Sep 2024 15:49:54 +0200 Subject: [PATCH 3/4] [#2723] Update django-admin-index fixture --- src/open_inwoner/conf/fixtures/django-admin-index.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/open_inwoner/conf/fixtures/django-admin-index.json b/src/open_inwoner/conf/fixtures/django-admin-index.json index fc3bde26b9..3baa74d121 100644 --- a/src/open_inwoner/conf/fixtures/django-admin-index.json +++ b/src/open_inwoner/conf/fixtures/django-admin-index.json @@ -293,11 +293,11 @@ "lapostaconfig" ], [ - "notifications_api_common", - "notificationsconfig" + "notifications", + "notificationsapiconfig" ], [ - "notifications_api_common", + "notifications", "subscription" ], [ From 912095b2e45924cdd340bde32bc6ac8a5dcfcea4 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Mon, 16 Sep 2024 12:42:44 +0200 Subject: [PATCH 4/4] [#2723] Check for errors in notification subscription creation --- src/notifications/models.py | 1 + src/open_inwoner/openzaak/auth.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/notifications/models.py b/src/notifications/models.py index 5d8730cccd..1200c48eba 100644 --- a/src/notifications/models.py +++ b/src/notifications/models.py @@ -131,6 +131,7 @@ def register(self) -> None: } response = client.post("abonnement", json=data) + response.raise_for_status() response_json = response.json() self._subscription = response_json["url"] diff --git a/src/open_inwoner/openzaak/auth.py b/src/open_inwoner/openzaak/auth.py index b7b95b8e6e..b0bc687a49 100644 --- a/src/open_inwoner/openzaak/auth.py +++ b/src/open_inwoner/openzaak/auth.py @@ -15,6 +15,9 @@ def get_valid_subscription_from_request(request) -> Subscription: auth_header = request.headers.get("Authorization") if not auth_header: + # notificaties.test.openzaak does not provide an auth header + # for test notifications when registering a webhook, so this + # can lead to false positives when testing raise InvalidAuth("missing Authorization header") return get_valid_subscriptions_from_bearer(auth_header)