From c55cae7b4ebb3f1393ea79de96ca6810524319cd Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Thu, 24 Oct 2024 09:54:50 +0200 Subject: [PATCH 01/38] :alien: Update NotificationsConfig client usage notifications-api-common has changed its underlying client to ape_pie.APIClient due to a zgw-consumers update (https://github.com/maykinmedia/notifications-api-common/pull/15) --- vng_api_common/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vng_api_common/views.py b/vng_api_common/views.py index 4edf4086..5ed40e94 100644 --- a/vng_api_common/views.py +++ b/vng_api_common/views.py @@ -199,7 +199,7 @@ def _test_nrc_config() -> list: error = False try: - nrc_client.list("kanaal") + nrc_client.get("kanaal") except requests.ConnectionError: error = True message = _("Could not connect with NRC") From dffe1e6e133757883f8b74eb63757c952e038c67 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 11:43:36 +0200 Subject: [PATCH 02/38] Update project requirements To make them match with notifications-api-common>=0.3.0 --- setup.cfg | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/setup.cfg b/setup.cfg index 75b70c68..82f5a6e4 100644 --- a/setup.cfg +++ b/setup.cfg @@ -13,7 +13,6 @@ keywords = openapi, swagger, django classifiers = Development Status :: 5 - Production/Stable Framework :: Django - Framework :: Django :: 3.2 Framework :: Django :: 4.2 Intended Audience :: Developers Operating System :: Unix @@ -34,18 +33,19 @@ scripts = bin/patch_content_types bin/use_external_components install_requires = - django>=3.2.0 - django-filter>=2.0 + django>=4.2.0 + django-filter>=23.1,<=25.1 django-solo - djangorestframework>=3.11.0 + djangorestframework>=3.15.0 djangorestframework_camel_case>=1.2.0 django-rest-framework-condition - drf-yasg>=1.20.0 - drf-nested-routers>=0.93.3 - gemma-zds-client>=0.14.0 + drf-yasg>=1.20.0 # TODO: remove usage? + drf-nested-routers>=0.94.1 iso-639 isodate - notifications-api-common>=0.2.2 + # TODO: create a data migration for + # APICredentials -> zgw_consumers.models.Service wherever applicable + notifications-api-common>=0.3.0 oyaml PyJWT>=2.0.0 requests From 1c8c547b874a7cc777383b7dbf339f54a2a98cd6 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 15:18:14 +0200 Subject: [PATCH 03/38] Add ape-pie & zgw-consumers requirements --- setup.cfg | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 82f5a6e4..d830209c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -43,13 +43,15 @@ install_requires = drf-nested-routers>=0.94.1 iso-639 isodate + notifications-api-common>=0.3.0 # TODO: create a data migration for # APICredentials -> zgw_consumers.models.Service wherever applicable - notifications-api-common>=0.3.0 + zgw-consumers>=0.35.1 oyaml PyJWT>=2.0.0 requests coreapi + ape-pie tests_require = pytest pytest-django From d1f6a092f54904856c9aa46033c0158db9eda195 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 15:20:23 +0200 Subject: [PATCH 04/38] Use to do external API calls --- vng_api_common/client.py | 75 ++++++++++++++++++------ vng_api_common/conf/api.py | 4 +- vng_api_common/models.py | 10 ++-- vng_api_common/notifications/handlers.py | 11 +++- vng_api_common/notifications/models.py | 3 +- vng_api_common/utils.py | 12 +++- vng_api_common/validators.py | 15 +++-- vng_api_common/views.py | 11 ++-- 8 files changed, 101 insertions(+), 40 deletions(-) diff --git a/vng_api_common/client.py b/vng_api_common/client.py index 3781806f..dbf6a391 100644 --- a/vng_api_common/client.py +++ b/vng_api_common/client.py @@ -2,16 +2,62 @@ Interface to get a zds_client object for a given URL. """ -from typing import Optional +import logging +from typing import Any, Optional -from django.apps import apps from django.conf import settings from django.utils.module_loading import import_string -from zds_client import Client +from ape_pie import APIClient +from requests import JSONDecodeError, RequestException, Response -def get_client(url: str, url_is_api_root=False) -> Optional[Client]: +logger = logging.getLogger(__name__) + + +class ClientError(RuntimeError): + pass + + +# TODO: use more approriate method name +def to_internal_data(response: Response) -> dict | list | None: + try: + response_json = response.json() + except JSONDecodeError: + logger.exception("Unable to parse json from response") + response_json = None + + try: + response.raise_for_status() + except RequestException as exc: + if response.status_code >= 500: + raise + raise ClientError(response_json if response_json is not None else {}) from exc + + assert response_json + return response_json + + +class Client(APIClient): + def request( + self, method: str | bytes, url: str | bytes, *args, **kwargs + ) -> Response: + + headers = kwargs.pop("headers", {}) + headers.setdefault("Accept", "application/json") + headers.setdefault("Content-Type", "application/json") + + kwargs["headers"] = headers + + data = kwargs.get("data", {}) + + if data: + kwargs["json"] = data + + return super().request(method, url, *args, **kwargs) + + +def get_client(url: str) -> Client | Any | None: """ Get a client instance for the given URL. @@ -25,20 +71,15 @@ def get_client(url: str, url_is_api_root=False) -> Optional[Client]: client_getter = import_string(custom_client_fetcher) return client_getter(url) - # default implementation - Client = import_string(settings.ZDS_CLIENT_CLASS) + from zgw_consumers.client import build_client + from zgw_consumers.models import Service - if url_is_api_root and not url.endswith("/"): - url = f"{url}/" + # default implementation + client_class = import_string(settings.CLIENT_CLASS) + service: Optional[Service] = Service.get_service(url) - client = Client.from_url(url) - if client is None: + if not service: + logger.warning(f"No service configured for {url}") return None - APICredential = apps.get_model("vng_api_common", "APICredential") - - if url_is_api_root: - client.base_url = url - - client.auth = APICredential.get_auth(url) - return client + return build_client(service.api_root, client_factory=client_class) diff --git a/vng_api_common/conf/api.py b/vng_api_common/conf/api.py index 4948bd5b..641fe22a 100644 --- a/vng_api_common/conf/api.py +++ b/vng_api_common/conf/api.py @@ -4,7 +4,7 @@ "BASE_SWAGGER_SETTINGS", "COMMON_SPEC", "LINK_FETCHER", - "ZDS_CLIENT_CLASS", + "CLIENT_CLASS", "GEMMA_URL_TEMPLATE", "GEMMA_URL_COMPONENTTYPE", "GEMMA_URL_INFORMATIEMODEL", @@ -94,7 +94,7 @@ # See: https://github.com/Rebilly/ReDoc#redoc-options-object LINK_FETCHER = "requests.get" -ZDS_CLIENT_CLASS = "zds_client.Client" +CLIENT_CLASS = "vng_api_common.client.Client" GEMMA_URL_TEMPLATE = "https://www.gemmaonline.nl/index.php/{informatiemodel}_{versie}/doc/{componenttype}/{component}" GEMMA_URL_COMPONENTTYPE = "objecttype" diff --git a/vng_api_common/models.py b/vng_api_common/models.py index 57ca2b5f..f5ab35af 100644 --- a/vng_api_common/models.py +++ b/vng_api_common/models.py @@ -1,13 +1,14 @@ -from typing import Optional, Union +from typing import Any, Optional, Union from urllib.parse import urlsplit, urlunsplit +from ape_pie.client import APIClient from django.db import models from django.db.models.functions import Length from django.utils.translation import gettext_lazy as _ from rest_framework.reverse import reverse from solo.models import SingletonModel -from zds_client import Client, ClientAuth +from zds_client import ClientAuth from .client import get_client as _get_client @@ -71,6 +72,7 @@ def __str__(self): return self.identifier +# TODO: remove this class class APICredential(models.Model): """ Store credentials for external APIs. @@ -166,9 +168,9 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) @classmethod - def get_client(cls) -> Optional[Client]: + def get_client(cls) -> APIClient | Any | None: """ Construct a client, prepared with the required auth. """ config = cls.get_solo() - return _get_client(config.api_root, url_is_api_root=True) + return _get_client(config.api_root) diff --git a/vng_api_common/notifications/handlers.py b/vng_api_common/notifications/handlers.py index 93342ec9..9366fc3f 100644 --- a/vng_api_common/notifications/handlers.py +++ b/vng_api_common/notifications/handlers.py @@ -4,7 +4,7 @@ from ..authorizations.models import Applicatie from ..authorizations.serializers import ApplicatieUuidSerializer -from ..client import get_client +from ..client import get_client, to_internal_data from ..constants import CommonResourceAction from ..utils import get_uuid_from_path @@ -20,8 +20,13 @@ def handle(self, message: dict) -> None: class AuthHandler: def _request_auth(self, url: str) -> dict: client = get_client(url) - response = client.retrieve("applicatie", url) - return underscoreize(response) + + if not client: + return {} + + response = client.get(url) + data = to_internal_data(response) + return underscoreize(data) def handle(self, message: dict) -> None: uuid = get_uuid_from_path(message["resource_url"]) diff --git a/vng_api_common/notifications/models.py b/vng_api_common/notifications/models.py index 4f987e50..f904458f 100644 --- a/vng_api_common/notifications/models.py +++ b/vng_api_common/notifications/models.py @@ -77,6 +77,7 @@ def register(self) -> None: client_id=self.client_id, secret=self.secret, ) + data = { "callbackUrl": self.callback_url, "auth": self_auth.credentials()["Authorization"], @@ -91,7 +92,7 @@ def register(self) -> None: } # register the subscriber - subscriber = client.create("abonnement", data=data) + subscriber = client.post("abonnement", data=data) self._subscription = subscriber["url"] self.save(update_fields=["_subscription"]) diff --git a/vng_api_common/utils.py b/vng_api_common/utils.py index c755e77a..c529edc9 100644 --- a/vng_api_common/utils.py +++ b/vng_api_common/utils.py @@ -12,10 +12,11 @@ from django.utils.encoding import smart_str from django.utils.module_loading import import_string +from requests import RequestException from rest_framework.utils import formatting from zds_client.client import ClientError -from .client import get_client +from .client import get_client, to_internal_data try: from djangorestframework_camel_case.util import ( @@ -191,9 +192,14 @@ def request_object_attribute( ) -> str: client = get_client(url) + if not client: + return "" + try: - result = client.retrieve(resource, url=url)[attribute] - except (ClientError, KeyError) as exc: + response = client.get(url) + data = to_internal_data(response) + result = data.get(attribute, "") if isinstance(data, dict) else "" + except RequestException as exc: logger.warning( "%s was retrieved from %s with the %s: %s", attribute, diff --git a/vng_api_common/validators.py b/vng_api_common/validators.py index a13286eb..1e3b5034 100644 --- a/vng_api_common/validators.py +++ b/vng_api_common/validators.py @@ -14,7 +14,7 @@ import requests from rest_framework import serializers, validators -from .client import get_client +from .client import get_client, to_internal_data from .constants import RSIN_LENGTH from .oas import fetcher, obj_has_shape @@ -240,15 +240,22 @@ def __call__(self, informatieobject: str, serializer): # dynamic so that it can be mocked in tests easily client = get_client(informatieobject) + if not client: + raise ValidationError( + _("Geen service geconfigureerd voor %s") % informatieobject + ) + try: - oios = client.list( + response = client.get( "objectinformatieobject", - query_params={ + params={ "informatieobject": informatieobject, "object": object_url, }, ) - except requests.HTTPError as exc: + + oios = to_internal_data(response) + except (requests.RequestException, RuntimeError) as exc: raise serializers.ValidationError( exc.args[0], code="relation-validation-error" ) from exc diff --git a/vng_api_common/views.py b/vng_api_common/views.py index 5ed40e94..33137129 100644 --- a/vng_api_common/views.py +++ b/vng_api_common/views.py @@ -129,7 +129,7 @@ def _test_ac_config() -> list: # check if AC auth is configured ac_client = AuthorizationsConfig.get_client() - has_ac_auth = ac_client.auth is not None + has_ac_auth = ac_client.auth is not None if ac_client else False checks = [ (_("Type of component"), auth_config.get_component_display(), None), @@ -146,10 +146,10 @@ def _test_ac_config() -> list: error = False try: - ac_client.list( - "applicatie", query_params={"clientIds": ac_client.auth.client_id} + ac_client.get( + "applicaties", params={"clientIds": ac_client.auth.client_id} ) - except requests.ConnectionError: + except requests.RequestException: error = True message = _("Could not connect with AC") except ClientError as exc: @@ -172,7 +172,6 @@ def _test_nrc_config() -> list: from notifications_api_common.models import NotificationsConfig, Subscription nrc_config = NotificationsConfig.get_solo() - nrc_client = NotificationsConfig.get_client() has_nrc_auth = nrc_client.auth is not None if nrc_client else False @@ -200,7 +199,7 @@ def _test_nrc_config() -> list: try: nrc_client.get("kanaal") - except requests.ConnectionError: + except requests.RequestException: error = True message = _("Could not connect with NRC") except ClientError as exc: From f24ee8f414d2222a8628111394fb394222037472 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 16:08:37 +0200 Subject: [PATCH 05/38] Remove APICredentials model --- setup.cfg | 2 - vng_api_common/admin.py | 21 +---- .../migrations/0006_delete_apicredential.py | 52 +++++++++++ vng_api_common/models.py | 86 +------------------ 4 files changed, 54 insertions(+), 107 deletions(-) create mode 100644 vng_api_common/migrations/0006_delete_apicredential.py diff --git a/setup.cfg b/setup.cfg index d830209c..5426e2d3 100644 --- a/setup.cfg +++ b/setup.cfg @@ -44,8 +44,6 @@ install_requires = iso-639 isodate notifications-api-common>=0.3.0 - # TODO: create a data migration for - # APICredentials -> zgw_consumers.models.Service wherever applicable zgw-consumers>=0.35.1 oyaml PyJWT>=2.0.0 diff --git a/vng_api_common/admin.py b/vng_api_common/admin.py index c67c1374..f5e7b477 100644 --- a/vng_api_common/admin.py +++ b/vng_api_common/admin.py @@ -1,29 +1,10 @@ from django.contrib import admin from django.utils.translation import gettext_lazy as _ -from .models import APICredential, JWTSecret +from .models import JWTSecret @admin.register(JWTSecret) class JWTSecretAdmin(admin.ModelAdmin): list_display = ("identifier",) search_fields = ("identifier",) - - -@admin.register(APICredential) -class APICredentialAdmin(admin.ModelAdmin): - list_display = ("label", "api_root", "client_id", "user_id") - search_fields = ("label", "api_root") - fieldsets = ( - (_("external API"), {"fields": ["api_root", "label"]}), - ( - _("credentials"), - { - "description": _( - "Credentials that indicate how this API or application identifies itself at the external " - "API." - ), - "fields": ["client_id", "secret", "user_id", "user_representation"], - }, - ), - ) diff --git a/vng_api_common/migrations/0006_delete_apicredential.py b/vng_api_common/migrations/0006_delete_apicredential.py new file mode 100644 index 00000000..2a92b39b --- /dev/null +++ b/vng_api_common/migrations/0006_delete_apicredential.py @@ -0,0 +1,52 @@ +# Generated by Django 5.1.2 on 2024-10-24 13:51 + +import logging + +from django.db import migrations + +from zgw_consumers.constants import APITypes, AuthTypes + + +logger = logging.getLogger(__name__) + + +def migrate_credentials_to_service(apps, _) -> None: + APICredential = apps.get_model("vng_api_common", "APICredential") + Service = apps.get_model("zgw_consumers", "Service") + + credentials = APICredential.objects.all() + + for credential in credentials: + logger.info("Creating Service for {credential.client_id") + + _, created = Service.objects.get_or_create( + api_root=credential.api_root, + defaults=dict( + label=credential.label, + api_type=APITypes.orc, + auth_type=AuthTypes.zgw, + client_id=credential.client_id, + secret=credential.secret, + user_id=credential.user_id, + user_representation=credential.user_representation, + ) + ) + + if created: + logger.info("Created new Service for {credential.api_root}") + else: + logger.info("Existing service found for {credential.api_root}") + + +class Migration(migrations.Migration): + + dependencies = [ + ("vng_api_common", "0005_auto_20190614_1346"), + ] + + operations = [ + migrations.RunPython(migrate_credentials_to_service), + migrations.DeleteModel( + name="APICredential", + ), + ] diff --git a/vng_api_common/models.py b/vng_api_common/models.py index f5ab35af..8b6862fe 100644 --- a/vng_api_common/models.py +++ b/vng_api_common/models.py @@ -1,14 +1,11 @@ -from typing import Any, Optional, Union -from urllib.parse import urlsplit, urlunsplit +from typing import Any from ape_pie.client import APIClient from django.db import models -from django.db.models.functions import Length from django.utils.translation import gettext_lazy as _ from rest_framework.reverse import reverse from solo.models import SingletonModel -from zds_client import ClientAuth from .client import get_client as _get_client @@ -72,87 +69,6 @@ def __str__(self): return self.identifier -# TODO: remove this class -class APICredential(models.Model): - """ - Store credentials for external APIs. - - When we need to authenticate against a remote API, we need to know which - client ID and secret to use to sign the JWT. - """ - - api_root = models.URLField( - _("API-root"), - unique=True, - help_text=_( - "URL of the external API, ending in a trailing slash. Example: https://example.com/api/v1/" - ), - ) - label = models.CharField( - _("label"), - max_length=100, - default="", - help_text=_("Human readable label of the external API."), - ) - client_id = models.CharField( - _("client ID"), - max_length=255, - help_text=_("Client ID to identify this API at the external API."), - ) - secret = models.CharField( - _("secret"), max_length=255, help_text=_("Secret belonging to the client ID.") - ) - user_id = models.CharField( - _("user ID"), - max_length=255, - help_text=_( - "User ID to use for the audit trail. Although these external API credentials are typically used by" - "this API itself instead of a user, the user ID is required." - ), - ) - user_representation = models.CharField( - _("user representation"), - max_length=255, - default="", - help_text=_("Human readable representation of the user."), - ) - - class Meta: - verbose_name = _("external API credential") - verbose_name_plural = _("external API credentials") - - def __str__(self): - return self.api_root - - @classmethod - def get_auth(cls, url: str, **kwargs) -> Union[ClientAuth, None]: - split_url = urlsplit(url) - scheme_and_domain = urlunsplit(split_url[:2] + ("", "", "")) - - candidates = ( - cls.objects.filter(api_root__startswith=scheme_and_domain) - .annotate(api_root_length=Length("api_root")) - .order_by("-api_root_length") - ) - - # select the one matching - for candidate in candidates.iterator(): - if url.startswith(candidate.api_root): - credentials = candidate - break - else: - return None - - auth = ClientAuth( - client_id=credentials.client_id, - secret=credentials.secret, - user_id=credentials.user_id, - user_representation=credentials.user_representation, - **kwargs, - ) - return auth - - class ClientConfig(SingletonModel): api_root = models.URLField(_("api root"), unique=True) From e131840cee8cffd7467631c4e4c2ba425e3a72c8 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 16:09:08 +0200 Subject: [PATCH 06/38] Remove zds_client usage --- tests/test_jwtsecrets.py | 9 +++---- vng_api_common/client.py | 32 +++++++++++++++++++++++- vng_api_common/mocks.py | 6 ++++- vng_api_common/notifications/models.py | 34 ++++++++++++++------------ vng_api_common/oas.py | 3 --- vng_api_common/routers.py | 6 ++--- vng_api_common/utils.py | 1 - vng_api_common/views.py | 3 ++- 8 files changed, 64 insertions(+), 30 deletions(-) diff --git a/tests/test_jwtsecrets.py b/tests/test_jwtsecrets.py index dd6e8880..5587ec10 100644 --- a/tests/test_jwtsecrets.py +++ b/tests/test_jwtsecrets.py @@ -2,13 +2,14 @@ from rest_framework import status from rest_framework.reverse import reverse from rest_framework.test import APIClient -from zds_client.auth import ClientAuth from vng_api_common.authorizations.models import Applicatie, Autorisatie +from vng_api_common.client import get_auth_headers from vng_api_common.constants import ComponentTypes from vng_api_common.models import JWTSecret + @pytest.mark.django_db def test_unauthorized_jwtsecret_create_forbidden(): url = reverse("jwtsecret-create") @@ -31,10 +32,8 @@ def test_authorized_jwtsecret_create_ok(): component=ComponentTypes.ac, scopes=["autorisaties.credentials-registreren"], ) - auth = ClientAuth(client_id="pytest", secret="sekrit").credentials()[ - "Authorization" - ] - client.credentials(HTTP_AUTHORIZATION=auth) + auth_headers = get_auth_headers("pytest", "sekrit") + client.credentials(HTTP_AUTHORIZATION=auth_headers["Authorization"]) response = client.post(url, {"identifier": "foo", "secret": "bar"}) diff --git a/vng_api_common/client.py b/vng_api_common/client.py index dbf6a391..a4f6f909 100644 --- a/vng_api_common/client.py +++ b/vng_api_common/client.py @@ -1,16 +1,21 @@ """ -Interface to get a zds_client object for a given URL. +Interface to get a client object for a given URL. """ import logging +import time from typing import Any, Optional from django.conf import settings from django.utils.module_loading import import_string +import jwt + from ape_pie import APIClient from requests import JSONDecodeError, RequestException, Response +JWT_ALG = "HS256" + logger = logging.getLogger(__name__) @@ -83,3 +88,28 @@ def get_client(url: str) -> Client | Any | None: return None return build_client(service.api_root, client_factory=client_class) + + +def get_auth_headers( + client_id: str, + client_secret: str, + user_id: str = "", + user_representation: str = "", + **claims +) -> dict: + payload = { + # standard claims + "iss": client_id, + "iat": int(time.time()), + # custom claims + "client_id": client_id, + "user_id": user_id, + "user_representation": user_representation, + **claims, + } + + encoded = jwt.encode(payload, client_secret, algorithm=JWT_ALG) + + return { + "Authorization": "Bearer {encoded}".format(encoded=encoded) + } diff --git a/vng_api_common/mocks.py b/vng_api_common/mocks.py index 4deb75e1..757b23ef 100644 --- a/vng_api_common/mocks.py +++ b/vng_api_common/mocks.py @@ -1,7 +1,11 @@ import re from urllib.parse import urlparse -from zds_client.client import UUID_PATTERN + +UUID_PATTERN = re.compile( + r"[0-9a-f]{8}\-[0-9a-f]{4}\-4[0-9a-f]{3}\-[89ab][0-9a-f]{3}\-[0-9a-f]{12}", + flags=re.I, +) class Response: diff --git a/vng_api_common/notifications/models.py b/vng_api_common/notifications/models.py index f904458f..1f721544 100644 --- a/vng_api_common/notifications/models.py +++ b/vng_api_common/notifications/models.py @@ -5,11 +5,11 @@ from django.db import models from django.utils.translation import gettext_lazy as _ -from zds_client import ClientAuth +from requests import RequestException -from ..client import get_client +from ..client import ClientError, get_auth_headers, get_client, to_internal_data from ..decorators import field_default -from ..models import APICredential, ClientConfig +from ..models import ClientConfig @field_default("api_root", "https://notificaties-api.vng.cloud/api/v1/") @@ -17,10 +17,6 @@ class NotificationsConfig(ClientConfig): class Meta: verbose_name = _("Notificatiescomponentconfiguratie") - def get_auth(self) -> ClientAuth: - auth = APICredential.get_auth(self.api_root) - return auth - class Subscription(models.Model): """ @@ -73,14 +69,11 @@ def register(self) -> None: # This authentication is for the NC to call us. Thus, it's *not* for # calling the NC to create a subscription. - self_auth = ClientAuth( - client_id=self.client_id, - secret=self.secret, - ) + self_auth = get_auth_headers(self.client_id, self.secret) data = { "callbackUrl": self.callback_url, - "auth": self_auth.credentials()["Authorization"], + "auth": self_auth["Authorization"], "kanalen": [ { "naam": channel, @@ -92,7 +85,18 @@ def register(self) -> None: } # register the subscriber - subscriber = client.post("abonnement", data=data) - - self._subscription = subscriber["url"] + try: + response = client.post("abonnement", data=data) + data = to_internal_data(response) + except RequestException: + raise RuntimeError(f"Failed adding subscription for {self.callback_url}") + except ClientError as exc: + response_json = exc.args[0] + + raise RuntimeError( + f"Failed adding subscription for {self.callback_url}. Invalid request" + f" data provided: {response_json}" + ) + + self._subscription = data["url"] self.save(update_fields=["_subscription"]) diff --git a/vng_api_common/oas.py b/vng_api_common/oas.py index 908e3766..3bbb7f3a 100644 --- a/vng_api_common/oas.py +++ b/vng_api_common/oas.py @@ -1,8 +1,5 @@ """ Utility module for Open API Specification 3.0.x. - -This should get merged into gemma-zds-client, but some heavy refactoring is -needed for that. """ from typing import Union diff --git a/vng_api_common/routers.py b/vng_api_common/routers.py index 159ecd58..0c821ebc 100644 --- a/vng_api_common/routers.py +++ b/vng_api_common/routers.py @@ -8,7 +8,7 @@ class APIRootView(_APIRootView): permission_classes = () -class ZDSNestedRegisteringMixin: +class NestedRegisteringMixin: _nested_router = None def __init__(self, *args, **kwargs): @@ -47,11 +47,11 @@ def register(self, prefix, viewset, nested=None, *args, **kwargs): ) -class NestedSimpleRouter(ZDSNestedRegisteringMixin, routers.NestedSimpleRouter): +class NestedSimpleRouter(NestedRegisteringMixin, routers.NestedSimpleRouter): pass -class DefaultRouter(ZDSNestedRegisteringMixin, routers.DefaultRouter): +class DefaultRouter(NestedRegisteringMixin, routers.DefaultRouter): APIRootView = APIRootView diff --git a/vng_api_common/utils.py b/vng_api_common/utils.py index c529edc9..91f836ec 100644 --- a/vng_api_common/utils.py +++ b/vng_api_common/utils.py @@ -14,7 +14,6 @@ from requests import RequestException from rest_framework.utils import formatting -from zds_client.client import ClientError from .client import get_client, to_internal_data diff --git a/vng_api_common/views.py b/vng_api_common/views.py index 33137129..1bb06b89 100644 --- a/vng_api_common/views.py +++ b/vng_api_common/views.py @@ -13,7 +13,8 @@ from rest_framework import exceptions as drf_exceptions, status from rest_framework.response import Response from rest_framework.views import exception_handler as drf_exception_handler -from zds_client import ClientError + +from vng_api_common.client import ClientError from . import exceptions from .compat import sentry_client From 15e7d3bbdb5d097ac64320941c58681b5125474c Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 16:13:18 +0200 Subject: [PATCH 07/38] Run formatting --- tests/test_jwtsecrets.py | 1 - vng_api_common/client.py | 7 ++----- vng_api_common/migrations/0006_delete_apicredential.py | 3 +-- vng_api_common/mocks.py | 1 - vng_api_common/models.py | 2 +- vng_api_common/views.py | 4 +--- 6 files changed, 5 insertions(+), 13 deletions(-) diff --git a/tests/test_jwtsecrets.py b/tests/test_jwtsecrets.py index 5587ec10..1fd5efe9 100644 --- a/tests/test_jwtsecrets.py +++ b/tests/test_jwtsecrets.py @@ -9,7 +9,6 @@ from vng_api_common.models import JWTSecret - @pytest.mark.django_db def test_unauthorized_jwtsecret_create_forbidden(): url = reverse("jwtsecret-create") diff --git a/vng_api_common/client.py b/vng_api_common/client.py index a4f6f909..91f7c28b 100644 --- a/vng_api_common/client.py +++ b/vng_api_common/client.py @@ -10,7 +10,6 @@ from django.utils.module_loading import import_string import jwt - from ape_pie import APIClient from requests import JSONDecodeError, RequestException, Response @@ -95,7 +94,7 @@ def get_auth_headers( client_secret: str, user_id: str = "", user_representation: str = "", - **claims + **claims, ) -> dict: payload = { # standard claims @@ -110,6 +109,4 @@ def get_auth_headers( encoded = jwt.encode(payload, client_secret, algorithm=JWT_ALG) - return { - "Authorization": "Bearer {encoded}".format(encoded=encoded) - } + return {"Authorization": "Bearer {encoded}".format(encoded=encoded)} diff --git a/vng_api_common/migrations/0006_delete_apicredential.py b/vng_api_common/migrations/0006_delete_apicredential.py index 2a92b39b..d4773134 100644 --- a/vng_api_common/migrations/0006_delete_apicredential.py +++ b/vng_api_common/migrations/0006_delete_apicredential.py @@ -6,7 +6,6 @@ from zgw_consumers.constants import APITypes, AuthTypes - logger = logging.getLogger(__name__) @@ -29,7 +28,7 @@ def migrate_credentials_to_service(apps, _) -> None: secret=credential.secret, user_id=credential.user_id, user_representation=credential.user_representation, - ) + ), ) if created: diff --git a/vng_api_common/mocks.py b/vng_api_common/mocks.py index 757b23ef..18c93bea 100644 --- a/vng_api_common/mocks.py +++ b/vng_api_common/mocks.py @@ -1,7 +1,6 @@ import re from urllib.parse import urlparse - UUID_PATTERN = re.compile( r"[0-9a-f]{8}\-[0-9a-f]{4}\-4[0-9a-f]{3}\-[89ab][0-9a-f]{3}\-[0-9a-f]{12}", flags=re.I, diff --git a/vng_api_common/models.py b/vng_api_common/models.py index 8b6862fe..6e922192 100644 --- a/vng_api_common/models.py +++ b/vng_api_common/models.py @@ -1,9 +1,9 @@ from typing import Any -from ape_pie.client import APIClient from django.db import models from django.utils.translation import gettext_lazy as _ +from ape_pie.client import APIClient from rest_framework.reverse import reverse from solo.models import SingletonModel diff --git a/vng_api_common/views.py b/vng_api_common/views.py index 1bb06b89..865b3a89 100644 --- a/vng_api_common/views.py +++ b/vng_api_common/views.py @@ -147,9 +147,7 @@ def _test_ac_config() -> list: error = False try: - ac_client.get( - "applicaties", params={"clientIds": ac_client.auth.client_id} - ) + ac_client.get("applicaties", params={"clientIds": ac_client.auth.client_id}) except requests.RequestException: error = True message = _("Could not connect with AC") From 4a628a10e28292179d0eca6b58c43922121ce15d Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 16:13:29 +0200 Subject: [PATCH 08/38] Update tox configuration --- tox.ini | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tox.ini b/tox.ini index a732e0e6..a65dae50 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,6 @@ [tox] envlist = - py{310,311}-django{32,42} + py{310,311}-django{42} isort docs black @@ -13,7 +13,6 @@ python = [gh-actions:env] DJANGO = - 3.2: django32 4.2: django42 [testenv] @@ -21,7 +20,6 @@ extras = tests coverage deps = - django32: Django~=3.2.0 django42: Django~=4.2.0 setenv = PYTHONPATH = {toxinidir} From d201498d5db645a9abbaaf52c8dfe4ba6f7374c3 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 16:18:04 +0200 Subject: [PATCH 09/38] Update documentation --- docs/ref/client.rst | 9 ++------- docs/ref/oas.rst | 11 ----------- .../management/commands/generate_swagger.py | 2 +- 3 files changed, 3 insertions(+), 19 deletions(-) delete mode 100644 docs/ref/oas.rst diff --git a/docs/ref/client.rst b/docs/ref/client.rst index 0496cbb7..c164577a 100644 --- a/docs/ref/client.rst +++ b/docs/ref/client.rst @@ -2,12 +2,7 @@ Obtaining a client ================== -Internally, `zds-client`_ is used to resolve remote API objects where the API is -documented using OpenAPI 3 specifications. - -Subclasses of the base ``Client`` class can also be used, in a pluggable fashion. By -default, the base class is used in combination with -:class:`vng_api_common.models.APICredential`. +Internally, the `APIClient`_ client is used to resolve remote API objects. Public API @@ -25,4 +20,4 @@ determined. :members: -.. _zds-client: https://pypi.org/project/gemma-zds-client/ +.. _APIClient: https://ape-pie.readthedocs.io/en/stable/reference.html#apiclient-class diff --git a/docs/ref/oas.rst b/docs/ref/oas.rst deleted file mode 100644 index 05579df3..00000000 --- a/docs/ref/oas.rst +++ /dev/null @@ -1,11 +0,0 @@ -============================== -OpenAPI Specification handling -============================== - -The processing of external OAS is limited to OAS 3.x. Most of the heavy lifting -to interact with remote APIs is done through the `zds-client`_. - -.. automodule:: vng_api_common.oas - :members: - -.. _zds-client: https://pypi.org/project/gemma-zds-client/ diff --git a/vng_api_common/management/commands/generate_swagger.py b/vng_api_common/management/commands/generate_swagger.py index 67942f7c..ebdd7101 100644 --- a/vng_api_common/management/commands/generate_swagger.py +++ b/vng_api_common/management/commands/generate_swagger.py @@ -47,7 +47,7 @@ def __init__( class Command(generate_swagger.Command): """ - Patches to the provided command to modify the schema for ZDS needs. + Patches to the provided command to modify the schema. """ leave_locale_alone = True From ab92b11819aee79a9d49aa40c233836a16dd0211 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 16:27:57 +0200 Subject: [PATCH 10/38] Add reverse migration path --- .../migrations/0006_delete_apicredential.py | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/vng_api_common/migrations/0006_delete_apicredential.py b/vng_api_common/migrations/0006_delete_apicredential.py index d4773134..3e4ceb59 100644 --- a/vng_api_common/migrations/0006_delete_apicredential.py +++ b/vng_api_common/migrations/0006_delete_apicredential.py @@ -32,9 +32,34 @@ def migrate_credentials_to_service(apps, _) -> None: ) if created: - logger.info("Created new Service for {credential.api_root}") + logger.info(f"Created new Service for {credential.api_root}") else: - logger.info("Existing service found for {credential.api_root}") + logger.info(f"Existing service found for {credential.api_root}") + + +def migrate_service_to_credentials(apps, _) -> None: + APICredential = apps.get_model("vng_api_common", "APICredential") + Service = apps.get_model("zgw_consumers", "Service") + + services = Service.objects.filter(auth_type=AuthTypes.zgw) + + for service in services: + logger.info("Creating APICredentials for {service.client_id") + + _, created = APICredential.objects.get_or_create( + api_root=service.api_root, + defaults=dict( + label=f"Migrated credentials for {service.client_id}", + client_id=service.client_id, + secret=service.secret, + user_id=service.user_id, + user_representation=service.user_representation, + ), + ) + if created: + logger.info(f"Created new APICredentials for {service.api_root}") + else: + logger.info(f"Existing APICredentials found for {service.api_root}") class Migration(migrations.Migration): @@ -44,7 +69,9 @@ class Migration(migrations.Migration): ] operations = [ - migrations.RunPython(migrate_credentials_to_service), + migrations.RunPython( + migrate_credentials_to_service, reverse_code=migrate_service_to_credentials + ), migrations.DeleteModel( name="APICredential", ), From b93738e5634eac59baf9770e6f7bf635ed993d34 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 16:28:06 +0200 Subject: [PATCH 11/38] Update docs --- docs/ref/index.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/ref/index.rst b/docs/ref/index.rst index 14845999..5216823b 100644 --- a/docs/ref/index.rst +++ b/docs/ref/index.rst @@ -18,6 +18,5 @@ Modules reference geo polymorphism client - oas utils testing From c79f7864ad9540c6c3b70e8b885ff45c8d41178d Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Thu, 24 Oct 2024 16:34:10 +0200 Subject: [PATCH 12/38] Update CI configuration --- .github/workflows/ci.yml | 4 ++-- .github/workflows/code_quality.yml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a80fe48d..295de77f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,7 @@ jobs: strategy: matrix: python: ['3.10', '3.11'] - django: ['3.2', '4.2'] + django: ['4.2'] services: postgres: image: postgres:14 @@ -62,7 +62,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: '3.9' + python-version: '3.10' - name: Build sdist and wheel run: | diff --git a/.github/workflows/code_quality.yml b/.github/workflows/code_quality.yml index acf2c3a0..8aecd141 100644 --- a/.github/workflows/code_quality.yml +++ b/.github/workflows/code_quality.yml @@ -28,7 +28,7 @@ jobs: - uses: actions/checkout@v2 - uses: actions/setup-python@v2 with: - python-version: '3.9' + python-version: '3.10' - name: Install dependencies run: pip install tox tox-gh-actions - run: tox From 8cd3bea83e175f7c293a88c3768f209304f416a1 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 10:37:50 +0200 Subject: [PATCH 13/38] Describe `APICredentials` to `Service` model migration --- docs/ref/client.rst | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/ref/client.rst b/docs/ref/client.rst index c164577a..c31bf089 100644 --- a/docs/ref/client.rst +++ b/docs/ref/client.rst @@ -2,7 +2,11 @@ Obtaining a client ================== -Internally, the `APIClient`_ client is used to resolve remote API objects. +Internally, the `APIClient`_ client is used to resolve remote API objects. To +allow the `APIClient`_ to correctly, e.g use correct credentials for authentication +, an `Service`_ instance is required with a matching `api_root`. This replaces +the previous mechanism to use `APICredentials` for this purpose. A data migration +will be performed to migrate `APICredentials` to the `Service`_ model. Public API @@ -21,3 +25,4 @@ determined. .. _APIClient: https://ape-pie.readthedocs.io/en/stable/reference.html#apiclient-class +.. _Service: https://zgw-consumers.readthedocs.io/en/latest/models.html#zgw_consumers.models.Service From fd16c375588a55fed88e8615527a78421d2e6ac0 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 11:10:20 +0200 Subject: [PATCH 14/38] Add missing field for Service's --- .../migrations/0006_delete_apicredential.py | 44 ++++++++++++------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/vng_api_common/migrations/0006_delete_apicredential.py b/vng_api_common/migrations/0006_delete_apicredential.py index 3e4ceb59..22c92a90 100644 --- a/vng_api_common/migrations/0006_delete_apicredential.py +++ b/vng_api_common/migrations/0006_delete_apicredential.py @@ -2,8 +2,9 @@ import logging -from django.db import migrations +from django.db import IntegrityError, migrations +from django.utils.text import slugify from zgw_consumers.constants import APITypes, AuthTypes logger = logging.getLogger(__name__) @@ -16,20 +17,31 @@ def migrate_credentials_to_service(apps, _) -> None: credentials = APICredential.objects.all() for credential in credentials: - logger.info("Creating Service for {credential.client_id") - - _, created = Service.objects.get_or_create( - api_root=credential.api_root, - defaults=dict( - label=credential.label, - api_type=APITypes.orc, - auth_type=AuthTypes.zgw, - client_id=credential.client_id, - secret=credential.secret, - user_id=credential.user_id, - user_representation=credential.user_representation, - ), - ) + logger.info(f"Creating Service for {credential.client_id}") + + service_slug = slugify(credential.label) + + try: + _, created = Service.objects.get_or_create( + api_root=credential.api_root, + defaults=dict( + label=credential.label, + slug=service_slug, + api_type=APITypes.orc, + auth_type=AuthTypes.zgw, + client_id=credential.client_id, + secret=credential.secret, + user_id=credential.user_id, + user_representation=credential.user_representation, + ), + ) + except IntegrityError: + logger.warning( + f"Unable to create Service for {credential.api_root}. Check the" + "`service slug` field on the existing Service's to verify an existing" + " Service already exists." + ) + continue if created: logger.info(f"Created new Service for {credential.api_root}") @@ -44,7 +56,7 @@ def migrate_service_to_credentials(apps, _) -> None: services = Service.objects.filter(auth_type=AuthTypes.zgw) for service in services: - logger.info("Creating APICredentials for {service.client_id") + logger.info(f"Creating APICredentials for {service.client_id}") _, created = APICredential.objects.get_or_create( api_root=service.api_root, From 4933a198e706ee531c5c9ed02b1fc066bb54e6af Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 12:09:44 +0200 Subject: [PATCH 15/38] Try to determine `api_type` from `api_root` This can be useful for Openzaak instances where these paths are used --- .../migrations/0006_delete_apicredential.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/vng_api_common/migrations/0006_delete_apicredential.py b/vng_api_common/migrations/0006_delete_apicredential.py index 22c92a90..bfb7dd46 100644 --- a/vng_api_common/migrations/0006_delete_apicredential.py +++ b/vng_api_common/migrations/0006_delete_apicredential.py @@ -10,6 +10,22 @@ logger = logging.getLogger(__name__) +def get_api_type(api_root: str) -> APITypes: + mapping = { + "/autorisaties/api/": APITypes.ac, + "/zaken/api/": APITypes.zrc, + "/catalogi/api/": APITypes.ztc, + "/documenten/api/": APITypes.drc, + "/besluiten/api/": APITypes.drc, + } + + for path, _type in mapping.items(): + if path in api_root.lower(): + return _type + + return APITypes.orc + + def migrate_credentials_to_service(apps, _) -> None: APICredential = apps.get_model("vng_api_common", "APICredential") Service = apps.get_model("zgw_consumers", "Service") @@ -27,7 +43,7 @@ def migrate_credentials_to_service(apps, _) -> None: defaults=dict( label=credential.label, slug=service_slug, - api_type=APITypes.orc, + api_type=get_api_type(credential.api_root), auth_type=AuthTypes.zgw, client_id=credential.client_id, secret=credential.secret, From 82786bf7d909d70ebd654c4d8a9010d5dcb65c1c Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 12:31:10 +0200 Subject: [PATCH 16/38] Reuse `client.auth` to retrieve token --- tests/test_jwtsecrets.py | 2 +- tests/utils.py | 28 ++++++++++++++++++++++++++ vng_api_common/client.py | 23 --------------------- vng_api_common/notifications/models.py | 6 +++--- 4 files changed, 32 insertions(+), 27 deletions(-) create mode 100644 tests/utils.py diff --git a/tests/test_jwtsecrets.py b/tests/test_jwtsecrets.py index 1fd5efe9..3990f09d 100644 --- a/tests/test_jwtsecrets.py +++ b/tests/test_jwtsecrets.py @@ -4,9 +4,9 @@ from rest_framework.test import APIClient from vng_api_common.authorizations.models import Applicatie, Autorisatie -from vng_api_common.client import get_auth_headers from vng_api_common.constants import ComponentTypes from vng_api_common.models import JWTSecret +from vng_api_common.tests.utils import get_auth_headers @pytest.mark.django_db diff --git a/tests/utils.py b/tests/utils.py new file mode 100644 index 00000000..a3ddec7d --- /dev/null +++ b/tests/utils.py @@ -0,0 +1,28 @@ +import time + +import jwt + +JWT_ALG = "HS256" + + +def get_auth_headers( + client_id: str, + client_secret: str, + user_id: str = "", + user_representation: str = "", + **claims, +) -> dict: + payload = { + # standard claims + "iss": client_id, + "iat": int(time.time()), + # custom claims + "client_id": client_id, + "user_id": user_id, + "user_representation": user_representation, + **claims, + } + + encoded = jwt.encode(payload, client_secret, algorithm=JWT_ALG) + + return {"Authorization": "Bearer {encoded}".format(encoded=encoded)} diff --git a/vng_api_common/client.py b/vng_api_common/client.py index 91f7c28b..267ebf85 100644 --- a/vng_api_common/client.py +++ b/vng_api_common/client.py @@ -87,26 +87,3 @@ def get_client(url: str) -> Client | Any | None: return None return build_client(service.api_root, client_factory=client_class) - - -def get_auth_headers( - client_id: str, - client_secret: str, - user_id: str = "", - user_representation: str = "", - **claims, -) -> dict: - payload = { - # standard claims - "iss": client_id, - "iat": int(time.time()), - # custom claims - "client_id": client_id, - "user_id": user_id, - "user_representation": user_representation, - **claims, - } - - encoded = jwt.encode(payload, client_secret, algorithm=JWT_ALG) - - return {"Authorization": "Bearer {encoded}".format(encoded=encoded)} diff --git a/vng_api_common/notifications/models.py b/vng_api_common/notifications/models.py index 1f721544..e0b8a91d 100644 --- a/vng_api_common/notifications/models.py +++ b/vng_api_common/notifications/models.py @@ -7,7 +7,7 @@ from requests import RequestException -from ..client import ClientError, get_auth_headers, get_client, to_internal_data +from ..client import ClientError, get_client, to_internal_data from ..decorators import field_default from ..models import ClientConfig @@ -69,11 +69,11 @@ def register(self) -> None: # This authentication is for the NC to call us. Thus, it's *not* for # calling the NC to create a subscription. - self_auth = get_auth_headers(self.client_id, self.secret) + token = getattr("_token", client.auth, "") if client and client.auth else "" data = { "callbackUrl": self.callback_url, - "auth": self_auth["Authorization"], + "auth": f"Bearer {token}", "kanalen": [ { "naam": channel, From 726b28a7ad2528ef8557c86213ace0979c3f0d2d Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 12:49:35 +0200 Subject: [PATCH 17/38] Remove oas related validators --- vng_api_common/oas.py | 94 ------------------------------------ vng_api_common/validators.py | 65 ------------------------- 2 files changed, 159 deletions(-) delete mode 100644 vng_api_common/oas.py diff --git a/vng_api_common/oas.py b/vng_api_common/oas.py deleted file mode 100644 index 3bbb7f3a..00000000 --- a/vng_api_common/oas.py +++ /dev/null @@ -1,94 +0,0 @@ -""" -Utility module for Open API Specification 3.0.x. -""" - -from typing import Union - -import requests -import yaml -from drf_yasg import openapi - -TYPE_MAP = { - openapi.TYPE_OBJECT: dict, - openapi.TYPE_STRING: str, - openapi.TYPE_NUMBER: (float, int), - openapi.TYPE_INTEGER: int, - openapi.TYPE_BOOLEAN: bool, - openapi.TYPE_ARRAY: list, -} - - -class SchemaFetcher: - def __init__(self): - self.cache = {} - - def fetch(self, url: str): - """ - Fetch a YAML-based OAS 3.0.x schema. - """ - if url in self.cache: - return self.cache[url] - - response = requests.get(url) - response.raise_for_status() - - spec = yaml.safe_load(response.content) - spec_version = response.headers.get( - "X-OAS-Version", spec.get("openapi", spec.get("swagger", "")) - ) - if not spec_version.startswith("3.0"): - raise ValueError("Unsupported spec version: {}".format(spec_version)) - - self.cache[url] = spec - return spec - - -def obj_has_shape(obj: Union[list, dict], schema: dict, resource: str) -> bool: - """ - Compare an instance of an object with the expected shape from an OAS 3 schema. - - ..todo:: doesn't handle references and nested schema's yet. - - :param obj: the value retrieved from the endpoint, json-decoded to a dict or list - :param schema: the OAS 3.0.x schema to test against, yaml-decoded to dict - :param resource: the name of the resource to test the schape against - """ - obj_schema = schema["components"]["schemas"][resource] - - required = obj_schema.get("required", []) - for prop, prop_schema in obj_schema["properties"].items(): - if prop in required and prop not in obj: - # missing required prop -> can't match the schema - return False - - # can't compare something that isn't there... - if prop not in obj: - continue - - value = obj[prop] - - # TODO Handling references not yet implemented - if "$ref" in prop_schema: - continue - - # TODO Handling allOf and oneOf not yet implemented - if "allOf" in prop_schema or "oneOf" in prop_schema: - continue - - # Allow None if property is nullable - if value is None: - if prop_schema.get("nullable", False): - continue - else: - return False - - expected_type = TYPE_MAP[prop_schema["type"]] - - # type mismatch -> not what we're looking for - if not isinstance(value, expected_type): - return False - - return True - - -fetcher = SchemaFetcher() diff --git a/vng_api_common/validators.py b/vng_api_common/validators.py index 1e3b5034..cbde1740 100644 --- a/vng_api_common/validators.py +++ b/vng_api_common/validators.py @@ -157,54 +157,6 @@ def __call__(self, value: str): return response -class ResourceValidator(URLValidator): - """ - Validate that the URL resolves to an instance of the external resource. - - :param resource: name of the resource, e.g. 'zaak' - :param oas_schema: URL to the schema to validate the response object shape - against. Must be a YAML OAS 3.0.x spec. - """ - - # Name mangling is applied to these attributes to avoid formatting issues - # that occur when overriding the superclass attributes - __message = _( - "The URL {url} resource did not look like a(n) `{resource}`. Please provide a valid URL." - ) - __code = "invalid-resource" - - def __init__(self, resource: str, oas_schema: str, *args, **kwargs): - self.resource = resource - self.oas_schema = oas_schema - super().__init__(*args, **kwargs) - - def __call__(self, url: str): - response = super().__call__(url) - - # at this point, we know the URL actually exists - try: - obj = response.json() - except json.JSONDecodeError as exc: - logger.info( - "URL %s doesn't seem to point to a JSON endpoint", url, exc_info=1 - ) - raise serializers.ValidationError( - self.__message.format(url=url, resource=self.resource), code=self.__code - ) - - # check if the shape matches - schema = fetcher.fetch(self.oas_schema) - if not obj_has_shape(obj, schema, self.resource): - logger.info( - "URL %s doesn't seem to point to a valid shape", url, exc_info=1 - ) - raise serializers.ValidationError( - self.__message.format(url=url, resource=self.resource), code=self.__code - ) - - return obj - - class InformatieObjectUniqueValidator(validators.UniqueTogetherValidator): requires_context = True @@ -380,20 +332,3 @@ def __call__(self, new_value, serializer_field): if new_value != current_value: raise serializers.ValidationError(self.message, code=self.code) - - -class PublishValidator(ResourceValidator): - """ - Validate that the URL actually resolves to a published resource (concept=False) - """ - - publish_message = _("The resource {url} is not published.") - publish_code = "not-published" - - def __call__(self, url: str): - response = super().__call__(url) - - if response.get("concept"): - raise serializers.ValidationError( - self.publish_message.format(url=url), code=self.publish_code - ) From 0649f9c0cabb344604915fc9d750475268f432dd Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 12:51:17 +0200 Subject: [PATCH 18/38] Fix `build_client` usage --- vng_api_common/client.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vng_api_common/client.py b/vng_api_common/client.py index 267ebf85..05d8aa91 100644 --- a/vng_api_common/client.py +++ b/vng_api_common/client.py @@ -13,7 +13,6 @@ from ape_pie import APIClient from requests import JSONDecodeError, RequestException, Response -JWT_ALG = "HS256" logger = logging.getLogger(__name__) @@ -86,4 +85,4 @@ def get_client(url: str) -> Client | Any | None: logger.warning(f"No service configured for {url}") return None - return build_client(service.api_root, client_factory=client_class) + return build_client(service, client_factory=client_class) From 47945363d4c7dc2dee6119374b547dca8ea98adb Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 12:53:19 +0200 Subject: [PATCH 19/38] Remove `request_object_attribute` --- vng_api_common/utils.py | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/vng_api_common/utils.py b/vng_api_common/utils.py index 91f836ec..d6df5c64 100644 --- a/vng_api_common/utils.py +++ b/vng_api_common/utils.py @@ -186,30 +186,6 @@ def get_uuid_from_path(path: str) -> str: return uuid_str -def request_object_attribute( - url: str, attribute: str, resource: Union[str, None] = None -) -> str: - client = get_client(url) - - if not client: - return "" - - try: - response = client.get(url) - data = to_internal_data(response) - result = data.get(attribute, "") if isinstance(data, dict) else "" - except RequestException as exc: - logger.warning( - "%s was retrieved from %s with the %s: %s", - attribute, - url, - exc.__class__.__name__, - exc, - ) - result = "" - return result - - def generate_unique_identification(instance: models.Model, date_field_name: str): model = type(instance) model_name = getattr(model, "IDENTIFICATIE_PREFIX", model._meta.model_name.upper()) From 9e3498a1a429673767b569d8f79d3d2f0376002b Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 15:13:58 +0200 Subject: [PATCH 20/38] Fix import error --- vng_api_common/validators.py | 1 - 1 file changed, 1 deletion(-) diff --git a/vng_api_common/validators.py b/vng_api_common/validators.py index cbde1740..c812aa53 100644 --- a/vng_api_common/validators.py +++ b/vng_api_common/validators.py @@ -16,7 +16,6 @@ from .client import get_client, to_internal_data from .constants import RSIN_LENGTH -from .oas import fetcher, obj_has_shape logger = logging.getLogger(__name__) From 9760dbb754559b98e691b0aa5c906b79bf9cec6c Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 15:18:27 +0200 Subject: [PATCH 21/38] Remove `notifications-api-common` related definitions These definitions are included in `notifications-api-common` --- tests/test_config.py | 13 --- vng_api_common/notifications/api/views.py | 3 +- vng_api_common/notifications/constants.py | 3 - vng_api_common/notifications/models.py | 102 ---------------------- 4 files changed, 2 insertions(+), 119 deletions(-) delete mode 100644 tests/test_config.py delete mode 100644 vng_api_common/notifications/constants.py delete mode 100644 vng_api_common/notifications/models.py diff --git a/tests/test_config.py b/tests/test_config.py deleted file mode 100644 index 35c29cf6..00000000 --- a/tests/test_config.py +++ /dev/null @@ -1,13 +0,0 @@ -from django.test import override_settings - -import pytest - -from vng_api_common.notifications.models import NotificationsConfig - - -@pytest.mark.django_db -@override_settings(CUSTOM_CLIENT_FETCHER="testapp.utils.get_client") -def test_notificationsconfig_custom_client(): - config = NotificationsConfig.get_solo() - client = config.get_client() - assert client == "testclient" diff --git a/vng_api_common/notifications/api/views.py b/vng_api_common/notifications/api/views.py index 6b392eb2..bf354c47 100644 --- a/vng_api_common/notifications/api/views.py +++ b/vng_api_common/notifications/api/views.py @@ -3,6 +3,8 @@ from drf_yasg.utils import swagger_auto_schema from notifications_api_common.api.serializers import NotificatieSerializer +from notifications_api_common.constants import SCOPE_NOTIFICATIES_PUBLICEREN_LABEL + from rest_framework import status from rest_framework.response import Response from rest_framework.views import APIView @@ -10,7 +12,6 @@ from ...permissions import AuthScopesRequired from ...scopes import Scope from ...serializers import FoutSerializer, ValidatieFoutSerializer -from ..constants import SCOPE_NOTIFICATIES_PUBLICEREN_LABEL class NotificationBaseView(APIView): diff --git a/vng_api_common/notifications/constants.py b/vng_api_common/notifications/constants.py deleted file mode 100644 index c35927ee..00000000 --- a/vng_api_common/notifications/constants.py +++ /dev/null @@ -1,3 +0,0 @@ -# Exernally defined scopes. -SCOPE_NOTIFICATIES_CONSUMEREN_LABEL = "notificaties.consumeren" -SCOPE_NOTIFICATIES_PUBLICEREN_LABEL = "notificaties.publiceren" diff --git a/vng_api_common/notifications/models.py b/vng_api_common/notifications/models.py deleted file mode 100644 index e0b8a91d..00000000 --- a/vng_api_common/notifications/models.py +++ /dev/null @@ -1,102 +0,0 @@ -import uuid -from urllib.parse import urljoin - -from django.contrib.postgres.fields import ArrayField -from django.db import models -from django.utils.translation import gettext_lazy as _ - -from requests import RequestException - -from ..client import ClientError, get_client, to_internal_data -from ..decorators import field_default -from ..models import ClientConfig - - -@field_default("api_root", "https://notificaties-api.vng.cloud/api/v1/") -class NotificationsConfig(ClientConfig): - class Meta: - verbose_name = _("Notificatiescomponentconfiguratie") - - -class Subscription(models.Model): - """ - A single subscription. - - TODO: on change/update, update the subscription - """ - - config = models.ForeignKey("NotificationsConfig", on_delete=models.CASCADE) - - 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. - """ - dummy_detail_url = urljoin(self.config.api_root, f"foo/{uuid.uuid4()}") - client = get_client(dummy_detail_url) - - # This authentication is for the NC to call us. Thus, it's *not* for - # calling the NC to create a subscription. - token = getattr("_token", client.auth, "") if client and client.auth else "" - - data = { - "callbackUrl": self.callback_url, - "auth": f"Bearer {token}", - "kanalen": [ - { - "naam": channel, - # FIXME: You need to be able to configure these. - "filters": {}, - } - for channel in self.channels - ], - } - - # register the subscriber - try: - response = client.post("abonnement", data=data) - data = to_internal_data(response) - except RequestException: - raise RuntimeError(f"Failed adding subscription for {self.callback_url}") - except ClientError as exc: - response_json = exc.args[0] - - raise RuntimeError( - f"Failed adding subscription for {self.callback_url}. Invalid request" - f" data provided: {response_json}" - ) - - self._subscription = data["url"] - self.save(update_fields=["_subscription"]) From e35bfbdfeb5cf2adfd34272625dfe7386cc5ac85 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 16:22:52 +0200 Subject: [PATCH 22/38] Remove notifications-api-common related models & update AuthorizationsConfig model Removed models should be imported from notifications-api-common --- vng_api_common/authorizations/admin.py | 2 +- ..._authorizationsconfig_api_root_and_more.py | 79 +++++++++++++++++++ vng_api_common/authorizations/models.py | 41 +++++++++- vng_api_common/client.py | 2 - vng_api_common/models.py | 29 ------- ...011_remove_subscription_config_and_more.py | 23 ++++++ vng_api_common/views.py | 10 ++- 7 files changed, 149 insertions(+), 37 deletions(-) create mode 100644 vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py create mode 100644 vng_api_common/notifications/migrations/0011_remove_subscription_config_and_more.py diff --git a/vng_api_common/authorizations/admin.py b/vng_api_common/authorizations/admin.py index d4a05448..22140046 100644 --- a/vng_api_common/authorizations/admin.py +++ b/vng_api_common/authorizations/admin.py @@ -7,7 +7,7 @@ @admin.register(AuthorizationsConfig) class AuthorizationsConfigAdmin(SingletonModelAdmin): - list_display = ("api_root", "component") + list_display = ("authorizations_api_service", "component") @admin.register(Autorisatie) diff --git a/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py b/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py new file mode 100644 index 00000000..581b74b9 --- /dev/null +++ b/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py @@ -0,0 +1,79 @@ +# Generated by Django 5.1.2 on 2024-10-25 14:07 + +import logging + +import django.db.models.deletion + +from django.db import migrations, models +from django.utils.text import slugify +from zgw_consumers.constants import APITypes, AuthTypes + + +logger = logging.getLogger(__name__) + + +def migrate_authorization_config_to_service(apps, _) -> None: + AuthorizationsConfig = apps.get_model( + "vng_api_common", "AuthorizationsConfig" + ) + Service = apps.get_model("zgw_consumers", "Service") + + service_label = "Authorization API service" + + config = AuthorizationsConfig.get_solo() + _, created = Service.objects.get_or_create( + api_root=config.api_root, + defaults=dict( + label="Authorization API service", + slug=slugify(service_label), + api_type=APITypes.ac, + auth_type=AuthTypes.zgw, + ), + ) + + if created: + logger.info(f"Created new Service for {config.api_root}") + else: + logger.info(f"Existing service found for {config.api_root}") + + +def migrate_authorization_config_to_config(apps, _) -> None: + AuthorizationsConfig = apps.get_model( + "vng_api_common", "AuthorizationsConfig" + ) + + config = AuthorizationsConfig.get_solo() + config.api_root = config.service.api_root + + config.save() + + +class Migration(migrations.Migration): + + dependencies = [ + ("authorizations", "0015_auto_20220318_1608"), + ("zgw_consumers", "0022_set_default_service_slug"), + ] + + operations = [ + migrations.AddField( + model_name="authorizationsconfig", + name="authorizations_api_service", + field=models.ForeignKey( + blank=True, + limit_choices_to={"api_type": "ac", "auth_type": "zgw"}, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="zgw_consumers.service", + verbose_name="autorisations api service", + ), + ), + migrations.RunPython( + migrate_authorization_config_to_service, + reverse_code=migrate_authorization_config_to_config + ), + migrations.RemoveField( + model_name="authorizationsconfig", + name="api_root", + ), + ] diff --git a/vng_api_common/authorizations/models.py b/vng_api_common/authorizations/models.py index 2a2f4ce3..b18b6758 100644 --- a/vng_api_common/authorizations/models.py +++ b/vng_api_common/authorizations/models.py @@ -1,17 +1,28 @@ +from typing import Optional import uuid from django.contrib.postgres.fields import ArrayField from django.db import models from django.utils.translation import gettext_lazy as _ +from solo.models import SingletonModel +from zgw_consumers.constants import APITypes, AuthTypes +from zgw_consumers.models import Service + +from vng_api_common.client import Client, get_client from ..constants import ComponentTypes, VertrouwelijkheidsAanduiding from ..decorators import field_default from ..fields import VertrouwelijkheidsAanduidingField -from ..models import APIMixin, ClientConfig +from ..models import APIMixin + + +class AuthorizationsConfigManager(models.Manager): + def get_queryset(self): + qs = super().get_queryset() + return qs.select_related("authorizations_api_service") -@field_default("api_root", "https://autorisaties-api.vng.cloud/api/v1") -class AuthorizationsConfig(ClientConfig): +class AuthorizationsConfig(SingletonModel): component = models.CharField( _("component"), max_length=50, @@ -19,9 +30,33 @@ class AuthorizationsConfig(ClientConfig): default=ComponentTypes.zrc, ) + authorizations_api_service = models.ForeignKey( + Service, + limit_choices_to=dict( + api_type=APITypes.ac, + auth_type=AuthTypes.zgw, + ), + verbose_name=_("autorisations api service"), + on_delete=models.SET_NULL, + blank=True, + null=True, + ) + + objects = AuthorizationsConfigManager() + class Meta: verbose_name = _("Autorisatiecomponentconfiguratie") + @classmethod + def get_client(cls) -> Optional[Client]: + """ + Construct a client, prepared with the required auth. + """ + config = cls.get_solo() + if config.authorizations_api_service: + return get_client(config.authorizations_api_service.api_root) + return None + class ApplicatieManager(models.Manager): def get_by_natural_key(self, uuid): diff --git a/vng_api_common/client.py b/vng_api_common/client.py index 05d8aa91..ed9d2d83 100644 --- a/vng_api_common/client.py +++ b/vng_api_common/client.py @@ -3,13 +3,11 @@ """ import logging -import time from typing import Any, Optional from django.conf import settings from django.utils.module_loading import import_string -import jwt from ape_pie import APIClient from requests import JSONDecodeError, RequestException, Response diff --git a/vng_api_common/models.py b/vng_api_common/models.py index 6e922192..84ff1402 100644 --- a/vng_api_common/models.py +++ b/vng_api_common/models.py @@ -1,13 +1,7 @@ -from typing import Any - from django.db import models from django.utils.translation import gettext_lazy as _ -from ape_pie.client import APIClient from rest_framework.reverse import reverse -from solo.models import SingletonModel - -from .client import get_client as _get_client class APIMixin: @@ -67,26 +61,3 @@ class Meta: def __str__(self): return self.identifier - - -class ClientConfig(SingletonModel): - api_root = models.URLField(_("api root"), unique=True) - - class Meta: - abstract = True - - def __str__(self): - return self.api_root - - def save(self, *args, **kwargs): - if not self.api_root.endswith("/"): - self.api_root = f"{self.api_root}/" - super().save(*args, **kwargs) - - @classmethod - def get_client(cls) -> APIClient | Any | None: - """ - Construct a client, prepared with the required auth. - """ - config = cls.get_solo() - return _get_client(config.api_root) diff --git a/vng_api_common/notifications/migrations/0011_remove_subscription_config_and_more.py b/vng_api_common/notifications/migrations/0011_remove_subscription_config_and_more.py new file mode 100644 index 00000000..314167ef --- /dev/null +++ b/vng_api_common/notifications/migrations/0011_remove_subscription_config_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 5.1.2 on 2024-10-25 14:07 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("notifications", "0010_auto_20220704_1419"), + ] + + operations = [ + migrations.RemoveField( + model_name="subscription", + name="config", + ), + migrations.DeleteModel( + name="NotificationsConfig", + ), + migrations.DeleteModel( + name="Subscription", + ), + ] diff --git a/vng_api_common/views.py b/vng_api_common/views.py index 865b3a89..1e910676 100644 --- a/vng_api_common/views.py +++ b/vng_api_common/views.py @@ -134,7 +134,11 @@ def _test_ac_config() -> list: checks = [ (_("Type of component"), auth_config.get_component_display(), None), - (_("AC"), auth_config.api_root, auth_config.api_root.endswith("/")), + ( + _("AC"), + auth_config.authorizations_api_service.api_root, + auth_config.authorizations_api_service.api_root.endswith("/") + ), ( _("Credentials for AC"), _("Configured") if has_ac_auth else _("Missing"), @@ -146,8 +150,10 @@ def _test_ac_config() -> list: if has_ac_auth: error = False + client_id = ac_client.auth.service.client_id + try: - ac_client.get("applicaties", params={"clientIds": ac_client.auth.client_id}) + ac_client.get("applicaties", params={"clientIds": client_id}) except requests.RequestException: error = True message = _("Could not connect with AC") From 417ab78352c096eca87c1009bc8c331262c19dee Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 17:03:49 +0200 Subject: [PATCH 23/38] Fix migrations, use simple `get_client` function, migrate AuthorizationsConfig --- docs/ref/client.rst | 15 ------- tests/test_jwtsecrets.py | 29 +++++++++++++- tests/test_publish_validator.py | 39 ------------------- tests/test_views.py | 11 ++++-- tests/utils.py | 28 ------------- ..._authorizationsconfig_api_root_and_more.py | 17 +++----- vng_api_common/authorizations/models.py | 3 +- vng_api_common/client.py | 17 +------- vng_api_common/conf/api.py | 3 -- .../migrations/0006_delete_apicredential.py | 2 +- vng_api_common/notifications/api/views.py | 1 - vng_api_common/views.py | 8 +++- 12 files changed, 53 insertions(+), 120 deletions(-) delete mode 100644 tests/test_publish_validator.py delete mode 100644 tests/utils.py diff --git a/docs/ref/client.rst b/docs/ref/client.rst index c31bf089..f3a2dc75 100644 --- a/docs/ref/client.rst +++ b/docs/ref/client.rst @@ -9,20 +9,5 @@ the previous mechanism to use `APICredentials` for this purpose. A data migratio will be performed to migrate `APICredentials` to the `Service`_ model. -Public API -========== - -Settings --------- - -The setting ``CUSTOM_CLIENT_FETCHER`` is a string with the dotted path to a callable -taking a single ``url`` string argument. The callable should return a ready-to-use -client instance for that particular URL, or ``None`` if no suitable client can be -determined. - -.. automodule:: vng_api_common.client - :members: - - .. _APIClient: https://ape-pie.readthedocs.io/en/stable/reference.html#apiclient-class .. _Service: https://zgw-consumers.readthedocs.io/en/latest/models.html#zgw_consumers.models.Service diff --git a/tests/test_jwtsecrets.py b/tests/test_jwtsecrets.py index 3990f09d..ef217275 100644 --- a/tests/test_jwtsecrets.py +++ b/tests/test_jwtsecrets.py @@ -1,3 +1,6 @@ +import time + +import jwt import pytest from rest_framework import status from rest_framework.reverse import reverse @@ -6,7 +9,31 @@ from vng_api_common.authorizations.models import Applicatie, Autorisatie from vng_api_common.constants import ComponentTypes from vng_api_common.models import JWTSecret -from vng_api_common.tests.utils import get_auth_headers + +JWT_ALG = "HS256" + + +def get_auth_headers( + client_id: str, + client_secret: str, + user_id: str = "", + user_representation: str = "", + **claims, +) -> dict: + payload = { + # standard claims + "iss": client_id, + "iat": int(time.time()), + # custom claims + "client_id": client_id, + "user_id": user_id, + "user_representation": user_representation, + **claims, + } + + encoded = jwt.encode(payload, client_secret, algorithm=JWT_ALG) + + return {"Authorization": "Bearer {encoded}".format(encoded=encoded)} @pytest.mark.django_db diff --git a/tests/test_publish_validator.py b/tests/test_publish_validator.py deleted file mode 100644 index 145d028d..00000000 --- a/tests/test_publish_validator.py +++ /dev/null @@ -1,39 +0,0 @@ -from unittest.mock import patch - -import pytest -import requests_mock -from rest_framework.serializers import ValidationError - -from vng_api_common.validators import PublishValidator - - -@patch("vng_api_common.validators.obj_has_shape", return_value=True) -@patch("vng_api_common.validators.fetcher") -def publish_validate(value, *mocks): - api_spec = "http://example.com/src/openapi.yaml" - validator = PublishValidator("Resource", api_spec, lambda x: {}) - - url = "http://example.com/src/resource/1" - with requests_mock.Mocker() as m: - m.get(url, json=value) - - result = validator(url) - - return result - - -def test_publish_validator_concept_true(): - with pytest.raises(ValidationError) as err: - publish_validate({"concept": True}) - - error = err.value.detail - - assert error[0].code == "not-published" - - -def test_publish_validator_concept_false(): - publish_validate({"concept": False}) - - -def test_publish_validator_concept_empty(): - publish_validate({}) diff --git a/tests/test_views.py b/tests/test_views.py index 1e6ad29b..fc343138 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1,18 +1,23 @@ -from django.test import override_settings +from unittest.mock import patch + from django.urls import reverse import pytest +from notifications_api_common.models import NotificationsConfig from rest_framework import status @pytest.mark.django_db -@override_settings(CUSTOM_CLIENT_FETCHER="testapp.utils.get_client_with_auth") def test_config_view(api_client): """ regression test for https://github.com/open-zaak/open-notificaties/issues/119 """ - path = reverse("view-config") + notifications_config = NotificationsConfig.get_solo() + notifications_config.notifications_api_service = None + notifications_config.save() + + path = reverse("view-config") response = api_client.get(path) assert response.status_code == status.HTTP_200_OK diff --git a/tests/utils.py b/tests/utils.py deleted file mode 100644 index a3ddec7d..00000000 --- a/tests/utils.py +++ /dev/null @@ -1,28 +0,0 @@ -import time - -import jwt - -JWT_ALG = "HS256" - - -def get_auth_headers( - client_id: str, - client_secret: str, - user_id: str = "", - user_representation: str = "", - **claims, -) -> dict: - payload = { - # standard claims - "iss": client_id, - "iat": int(time.time()), - # custom claims - "client_id": client_id, - "user_id": user_id, - "user_representation": user_representation, - **claims, - } - - encoded = jwt.encode(payload, client_secret, algorithm=JWT_ALG) - - return {"Authorization": "Bearer {encoded}".format(encoded=encoded)} diff --git a/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py b/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py index 581b74b9..1ebde0f3 100644 --- a/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py +++ b/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py @@ -3,24 +3,21 @@ import logging import django.db.models.deletion - from django.db import migrations, models from django.utils.text import slugify -from zgw_consumers.constants import APITypes, AuthTypes +from zgw_consumers.constants import APITypes, AuthTypes logger = logging.getLogger(__name__) def migrate_authorization_config_to_service(apps, _) -> None: - AuthorizationsConfig = apps.get_model( - "vng_api_common", "AuthorizationsConfig" - ) + AuthorizationsConfig = apps.get_model("authorizations", "AuthorizationsConfig") Service = apps.get_model("zgw_consumers", "Service") service_label = "Authorization API service" - config = AuthorizationsConfig.get_solo() + config, _ = AuthorizationsConfig.objects.get_or_create() _, created = Service.objects.get_or_create( api_root=config.api_root, defaults=dict( @@ -38,11 +35,9 @@ def migrate_authorization_config_to_service(apps, _) -> None: def migrate_authorization_config_to_config(apps, _) -> None: - AuthorizationsConfig = apps.get_model( - "vng_api_common", "AuthorizationsConfig" - ) + AuthorizationsConfig = apps.get_model("authorizations", "AuthorizationsConfig") - config = AuthorizationsConfig.get_solo() + config, _ = AuthorizationsConfig.objects.get_or_create() config.api_root = config.service.api_root config.save() @@ -70,7 +65,7 @@ class Migration(migrations.Migration): ), migrations.RunPython( migrate_authorization_config_to_service, - reverse_code=migrate_authorization_config_to_config + reverse_code=migrate_authorization_config_to_config, ), migrations.RemoveField( model_name="authorizationsconfig", diff --git a/vng_api_common/authorizations/models.py b/vng_api_common/authorizations/models.py index b18b6758..bb795153 100644 --- a/vng_api_common/authorizations/models.py +++ b/vng_api_common/authorizations/models.py @@ -1,9 +1,10 @@ -from typing import Optional import uuid +from typing import Optional from django.contrib.postgres.fields import ArrayField from django.db import models from django.utils.translation import gettext_lazy as _ + from solo.models import SingletonModel from zgw_consumers.constants import APITypes, AuthTypes from zgw_consumers.models import Service diff --git a/vng_api_common/client.py b/vng_api_common/client.py index ed9d2d83..a92c0de2 100644 --- a/vng_api_common/client.py +++ b/vng_api_common/client.py @@ -11,8 +11,6 @@ from ape_pie import APIClient from requests import JSONDecodeError, RequestException, Response - - logger = logging.getLogger(__name__) @@ -58,29 +56,18 @@ def request( return super().request(method, url, *args, **kwargs) -def get_client(url: str) -> Client | Any | None: +def get_client(url: str) -> Client | None: """ Get a client instance for the given URL. - - If the setting CUSTOM_CLIENT_FETCHER is defined, then this callable is invoked. - Otherwise we fall back on the default implementation. - If no suitable client is found, ``None`` is returned. """ - custom_client_fetcher = getattr(settings, "CUSTOM_CLIENT_FETCHER", None) - if custom_client_fetcher: - client_getter = import_string(custom_client_fetcher) - return client_getter(url) - from zgw_consumers.client import build_client from zgw_consumers.models import Service - # default implementation - client_class = import_string(settings.CLIENT_CLASS) service: Optional[Service] = Service.get_service(url) if not service: logger.warning(f"No service configured for {url}") return None - return build_client(service, client_factory=client_class) + return build_client(service, client_factory=Client) diff --git a/vng_api_common/conf/api.py b/vng_api_common/conf/api.py index 641fe22a..d2156f2e 100644 --- a/vng_api_common/conf/api.py +++ b/vng_api_common/conf/api.py @@ -4,7 +4,6 @@ "BASE_SWAGGER_SETTINGS", "COMMON_SPEC", "LINK_FETCHER", - "CLIENT_CLASS", "GEMMA_URL_TEMPLATE", "GEMMA_URL_COMPONENTTYPE", "GEMMA_URL_INFORMATIEMODEL", @@ -94,8 +93,6 @@ # See: https://github.com/Rebilly/ReDoc#redoc-options-object LINK_FETCHER = "requests.get" -CLIENT_CLASS = "vng_api_common.client.Client" - GEMMA_URL_TEMPLATE = "https://www.gemmaonline.nl/index.php/{informatiemodel}_{versie}/doc/{componenttype}/{component}" GEMMA_URL_COMPONENTTYPE = "objecttype" GEMMA_URL_INFORMATIEMODEL = "Rgbz" diff --git a/vng_api_common/migrations/0006_delete_apicredential.py b/vng_api_common/migrations/0006_delete_apicredential.py index bfb7dd46..5318e50b 100644 --- a/vng_api_common/migrations/0006_delete_apicredential.py +++ b/vng_api_common/migrations/0006_delete_apicredential.py @@ -3,8 +3,8 @@ import logging from django.db import IntegrityError, migrations - from django.utils.text import slugify + from zgw_consumers.constants import APITypes, AuthTypes logger = logging.getLogger(__name__) diff --git a/vng_api_common/notifications/api/views.py b/vng_api_common/notifications/api/views.py index bf354c47..ba4f2f69 100644 --- a/vng_api_common/notifications/api/views.py +++ b/vng_api_common/notifications/api/views.py @@ -4,7 +4,6 @@ from drf_yasg.utils import swagger_auto_schema from notifications_api_common.api.serializers import NotificatieSerializer from notifications_api_common.constants import SCOPE_NOTIFICATIES_PUBLICEREN_LABEL - from rest_framework import status from rest_framework.response import Response from rest_framework.views import APIView diff --git a/vng_api_common/views.py b/vng_api_common/views.py index 1e910676..7d04887c 100644 --- a/vng_api_common/views.py +++ b/vng_api_common/views.py @@ -136,8 +136,12 @@ def _test_ac_config() -> list: (_("Type of component"), auth_config.get_component_display(), None), ( _("AC"), - auth_config.authorizations_api_service.api_root, - auth_config.authorizations_api_service.api_root.endswith("/") + ( + auth_config.authorizations_api_service.api_root + if ac_client + else _("Missing") + ), + bool(ac_client), ), ( _("Credentials for AC"), From 91744ee1ca308339bb21ad9640015ebff88c30a4 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 17:16:35 +0200 Subject: [PATCH 24/38] Update JWTAuth error handling --- vng_api_common/authorizations/middleware.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/vng_api_common/authorizations/middleware.py b/vng_api_common/authorizations/middleware.py index 9f1ebf5e..d0c05f10 100644 --- a/vng_api_common/authorizations/middleware.py +++ b/vng_api_common/authorizations/middleware.py @@ -8,9 +8,10 @@ import jwt from djangorestframework_camel_case.util import underscoreize +from requests import RequestException from rest_framework.exceptions import PermissionDenied -from zds_client.client import ClientError +from vng_api_common.client import ClientError, to_internal_data from vng_api_common.constants import VertrouwelijkheidsAanduiding from ..models import JWTSecret @@ -51,10 +52,18 @@ def autorisaties(self) -> models.QuerySet: def _request_auth(self) -> list: client = AuthorizationsConfig.get_client() + + if not client: + logger.warning("Authorization component can't be accessed") + return [] + try: - response = client.list( - "applicatie", query_params={"clientIds": self.client_id} - ) + response = client.get("applicaties", params={"clientIds": self.client_id}) + + data = to_internal_data(response) + except RequestException: + logger.warning("Authorization component can't be accessed") + return [] except ClientError as exc: response = exc.args[0] # friendly debug - hint at where the problem is located @@ -64,10 +73,10 @@ def _request_auth(self) -> list: "authorizations could not be retrieved" ) raise PermissionDenied(detail=detail, code="not_authenticated_for_ac") - logger.warn("Authorization component can't be accessed") + logger.warning("Authorization component can't be accessed") return [] - return underscoreize(response["results"]) + return underscoreize(data["results"]) def _get_auth(self): return Applicatie.objects.filter(client_ids__contains=[self.client_id]) From f8857cc9e1b6277744dedf8bd6d0421b6a5c7e08 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 25 Oct 2024 17:21:34 +0200 Subject: [PATCH 25/38] Fix import errors --- vng_api_common/authorizations/models.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vng_api_common/authorizations/models.py b/vng_api_common/authorizations/models.py index bb795153..ee18e6f1 100644 --- a/vng_api_common/authorizations/models.py +++ b/vng_api_common/authorizations/models.py @@ -7,7 +7,6 @@ from solo.models import SingletonModel from zgw_consumers.constants import APITypes, AuthTypes -from zgw_consumers.models import Service from vng_api_common.client import Client, get_client @@ -32,7 +31,7 @@ class AuthorizationsConfig(SingletonModel): ) authorizations_api_service = models.ForeignKey( - Service, + "zgw_consumers.Service", limit_choices_to=dict( api_type=APITypes.ac, auth_type=AuthTypes.zgw, From 475f5f55016e613252c6b39f352a76b983f47646 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Thu, 31 Oct 2024 11:41:43 +0100 Subject: [PATCH 26/38] Revert "Remove oas related validators" This reverts commit 726b28a7ad2528ef8557c86213ace0979c3f0d2d. --- vng_api_common/oas.py | 94 ++++++++++++++++++++++++++++++++++++ vng_api_common/validators.py | 65 +++++++++++++++++++++++++ 2 files changed, 159 insertions(+) create mode 100644 vng_api_common/oas.py diff --git a/vng_api_common/oas.py b/vng_api_common/oas.py new file mode 100644 index 00000000..3bbb7f3a --- /dev/null +++ b/vng_api_common/oas.py @@ -0,0 +1,94 @@ +""" +Utility module for Open API Specification 3.0.x. +""" + +from typing import Union + +import requests +import yaml +from drf_yasg import openapi + +TYPE_MAP = { + openapi.TYPE_OBJECT: dict, + openapi.TYPE_STRING: str, + openapi.TYPE_NUMBER: (float, int), + openapi.TYPE_INTEGER: int, + openapi.TYPE_BOOLEAN: bool, + openapi.TYPE_ARRAY: list, +} + + +class SchemaFetcher: + def __init__(self): + self.cache = {} + + def fetch(self, url: str): + """ + Fetch a YAML-based OAS 3.0.x schema. + """ + if url in self.cache: + return self.cache[url] + + response = requests.get(url) + response.raise_for_status() + + spec = yaml.safe_load(response.content) + spec_version = response.headers.get( + "X-OAS-Version", spec.get("openapi", spec.get("swagger", "")) + ) + if not spec_version.startswith("3.0"): + raise ValueError("Unsupported spec version: {}".format(spec_version)) + + self.cache[url] = spec + return spec + + +def obj_has_shape(obj: Union[list, dict], schema: dict, resource: str) -> bool: + """ + Compare an instance of an object with the expected shape from an OAS 3 schema. + + ..todo:: doesn't handle references and nested schema's yet. + + :param obj: the value retrieved from the endpoint, json-decoded to a dict or list + :param schema: the OAS 3.0.x schema to test against, yaml-decoded to dict + :param resource: the name of the resource to test the schape against + """ + obj_schema = schema["components"]["schemas"][resource] + + required = obj_schema.get("required", []) + for prop, prop_schema in obj_schema["properties"].items(): + if prop in required and prop not in obj: + # missing required prop -> can't match the schema + return False + + # can't compare something that isn't there... + if prop not in obj: + continue + + value = obj[prop] + + # TODO Handling references not yet implemented + if "$ref" in prop_schema: + continue + + # TODO Handling allOf and oneOf not yet implemented + if "allOf" in prop_schema or "oneOf" in prop_schema: + continue + + # Allow None if property is nullable + if value is None: + if prop_schema.get("nullable", False): + continue + else: + return False + + expected_type = TYPE_MAP[prop_schema["type"]] + + # type mismatch -> not what we're looking for + if not isinstance(value, expected_type): + return False + + return True + + +fetcher = SchemaFetcher() diff --git a/vng_api_common/validators.py b/vng_api_common/validators.py index c812aa53..f62b6a88 100644 --- a/vng_api_common/validators.py +++ b/vng_api_common/validators.py @@ -156,6 +156,54 @@ def __call__(self, value: str): return response +class ResourceValidator(URLValidator): + """ + Validate that the URL resolves to an instance of the external resource. + + :param resource: name of the resource, e.g. 'zaak' + :param oas_schema: URL to the schema to validate the response object shape + against. Must be a YAML OAS 3.0.x spec. + """ + + # Name mangling is applied to these attributes to avoid formatting issues + # that occur when overriding the superclass attributes + __message = _( + "The URL {url} resource did not look like a(n) `{resource}`. Please provide a valid URL." + ) + __code = "invalid-resource" + + def __init__(self, resource: str, oas_schema: str, *args, **kwargs): + self.resource = resource + self.oas_schema = oas_schema + super().__init__(*args, **kwargs) + + def __call__(self, url: str): + response = super().__call__(url) + + # at this point, we know the URL actually exists + try: + obj = response.json() + except json.JSONDecodeError as exc: + logger.info( + "URL %s doesn't seem to point to a JSON endpoint", url, exc_info=1 + ) + raise serializers.ValidationError( + self.__message.format(url=url, resource=self.resource), code=self.__code + ) + + # check if the shape matches + schema = fetcher.fetch(self.oas_schema) + if not obj_has_shape(obj, schema, self.resource): + logger.info( + "URL %s doesn't seem to point to a valid shape", url, exc_info=1 + ) + raise serializers.ValidationError( + self.__message.format(url=url, resource=self.resource), code=self.__code + ) + + return obj + + class InformatieObjectUniqueValidator(validators.UniqueTogetherValidator): requires_context = True @@ -331,3 +379,20 @@ def __call__(self, new_value, serializer_field): if new_value != current_value: raise serializers.ValidationError(self.message, code=self.code) + + +class PublishValidator(ResourceValidator): + """ + Validate that the URL actually resolves to a published resource (concept=False) + """ + + publish_message = _("The resource {url} is not published.") + publish_code = "not-published" + + def __call__(self, url: str): + response = super().__call__(url) + + if response.get("concept"): + raise serializers.ValidationError( + self.publish_message.format(url=url), code=self.publish_code + ) From f66f868a132e637feda363a19e06f771b153a749 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Fri, 8 Nov 2024 10:27:27 +0100 Subject: [PATCH 27/38] :memo: Re-add OAS utils docs --- docs/ref/index.rst | 1 + docs/ref/oas.rst | 9 +++++++++ 2 files changed, 10 insertions(+) create mode 100644 docs/ref/oas.rst diff --git a/docs/ref/index.rst b/docs/ref/index.rst index 5216823b..14845999 100644 --- a/docs/ref/index.rst +++ b/docs/ref/index.rst @@ -18,5 +18,6 @@ Modules reference geo polymorphism client + oas utils testing diff --git a/docs/ref/oas.rst b/docs/ref/oas.rst new file mode 100644 index 00000000..f7657baa --- /dev/null +++ b/docs/ref/oas.rst @@ -0,0 +1,9 @@ +============================== +OpenAPI Specification handling +============================== + +The processing of external OAS is limited to OAS 3.x. This module provides utilities to +perform operations on OAS. + +.. automodule:: vng_api_common.oas + :members: From 6c5af46c7000d8ddce890419e96deb5fc831096d Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Fri, 8 Nov 2024 10:35:02 +0100 Subject: [PATCH 28/38] :bug: Fix publishvalidator by adding missing imports the imports and the publishvalidator tests were removed --- tests/test_publish_validators.py | 39 ++++++++++++++++++++++++++++++++ vng_api_common/validators.py | 1 + 2 files changed, 40 insertions(+) create mode 100644 tests/test_publish_validators.py diff --git a/tests/test_publish_validators.py b/tests/test_publish_validators.py new file mode 100644 index 00000000..145d028d --- /dev/null +++ b/tests/test_publish_validators.py @@ -0,0 +1,39 @@ +from unittest.mock import patch + +import pytest +import requests_mock +from rest_framework.serializers import ValidationError + +from vng_api_common.validators import PublishValidator + + +@patch("vng_api_common.validators.obj_has_shape", return_value=True) +@patch("vng_api_common.validators.fetcher") +def publish_validate(value, *mocks): + api_spec = "http://example.com/src/openapi.yaml" + validator = PublishValidator("Resource", api_spec, lambda x: {}) + + url = "http://example.com/src/resource/1" + with requests_mock.Mocker() as m: + m.get(url, json=value) + + result = validator(url) + + return result + + +def test_publish_validator_concept_true(): + with pytest.raises(ValidationError) as err: + publish_validate({"concept": True}) + + error = err.value.detail + + assert error[0].code == "not-published" + + +def test_publish_validator_concept_false(): + publish_validate({"concept": False}) + + +def test_publish_validator_concept_empty(): + publish_validate({}) diff --git a/vng_api_common/validators.py b/vng_api_common/validators.py index f62b6a88..1e3b5034 100644 --- a/vng_api_common/validators.py +++ b/vng_api_common/validators.py @@ -16,6 +16,7 @@ from .client import get_client, to_internal_data from .constants import RSIN_LENGTH +from .oas import fetcher, obj_has_shape logger = logging.getLogger(__name__) From 8de8a6c72dd294c3790aa4ffe6ba10c8caece198 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Fri, 8 Nov 2024 10:39:11 +0100 Subject: [PATCH 29/38] :recycle: Replace get_auth_headers with generate_jwt util which can be reused in Open Zaak/Open Notificaties --- tests/test_jwtsecrets.py | 33 +++----------------------- vng_api_common/authorizations/utils.py | 17 +++++++++++++ 2 files changed, 20 insertions(+), 30 deletions(-) create mode 100644 vng_api_common/authorizations/utils.py diff --git a/tests/test_jwtsecrets.py b/tests/test_jwtsecrets.py index ef217275..b7ee143b 100644 --- a/tests/test_jwtsecrets.py +++ b/tests/test_jwtsecrets.py @@ -1,40 +1,13 @@ -import time - -import jwt import pytest from rest_framework import status from rest_framework.reverse import reverse from rest_framework.test import APIClient from vng_api_common.authorizations.models import Applicatie, Autorisatie +from vng_api_common.authorizations.utils import generate_jwt from vng_api_common.constants import ComponentTypes from vng_api_common.models import JWTSecret -JWT_ALG = "HS256" - - -def get_auth_headers( - client_id: str, - client_secret: str, - user_id: str = "", - user_representation: str = "", - **claims, -) -> dict: - payload = { - # standard claims - "iss": client_id, - "iat": int(time.time()), - # custom claims - "client_id": client_id, - "user_id": user_id, - "user_representation": user_representation, - **claims, - } - - encoded = jwt.encode(payload, client_secret, algorithm=JWT_ALG) - - return {"Authorization": "Bearer {encoded}".format(encoded=encoded)} - @pytest.mark.django_db def test_unauthorized_jwtsecret_create_forbidden(): @@ -58,8 +31,8 @@ def test_authorized_jwtsecret_create_ok(): component=ComponentTypes.ac, scopes=["autorisaties.credentials-registreren"], ) - auth_headers = get_auth_headers("pytest", "sekrit") - client.credentials(HTTP_AUTHORIZATION=auth_headers["Authorization"]) + token = generate_jwt("pytest", "sekrit", "pytest", "pytest") + client.credentials(HTTP_AUTHORIZATION=token) response = client.post(url, {"identifier": "foo", "secret": "bar"}) diff --git a/vng_api_common/authorizations/utils.py b/vng_api_common/authorizations/utils.py new file mode 100644 index 00000000..37e06af1 --- /dev/null +++ b/vng_api_common/authorizations/utils.py @@ -0,0 +1,17 @@ +def generate_jwt(client_id, secret, user_id, user_representation): + from zgw_consumers.client import ZGWAuth + + class FakeService: + def __init__(self, **kwargs): + for key, value in kwargs.items(): + setattr(self, key, value) + + auth = ZGWAuth( + service=FakeService( # type: ignore + client_id=client_id, + secret=secret, + user_id=user_id, + user_representation=user_representation, + ) + ) + return f"Bearer {auth._token}" From f3fe4ee5187f4b340f65ee1270bbe34024e213f3 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Fri, 8 Nov 2024 11:18:13 +0100 Subject: [PATCH 30/38] :white_check_mark: Add migration tests for service migrations --- tests/test_migrations.py | 176 ++++++++++++++++++ ..._authorizationsconfig_api_root_and_more.py | 8 +- 2 files changed, 181 insertions(+), 3 deletions(-) create mode 100644 tests/test_migrations.py diff --git a/tests/test_migrations.py b/tests/test_migrations.py new file mode 100644 index 00000000..5323f1d3 --- /dev/null +++ b/tests/test_migrations.py @@ -0,0 +1,176 @@ +from django.apps import apps as global_apps +from django.db import connection +from django.db.migrations.executor import MigrationExecutor +from django.test import override_settings + +import pytest +from zgw_consumers.constants import APITypes, AuthTypes + + +class BaseMigrationTest: + app = None + migrate_from = None # The migration before the one we want to test + migrate_to = None # The migration we want to test + setting_overrides = {} + + @pytest.fixture(autouse=True) + def setup_migration(self, db): + """ + Setup the migration test by reversing to `migrate_from` state, + then applying the `migrate_to` state. + """ + assert self.app is not None, "You must define the `app` attribute" + assert self.migrate_from is not None, "You must define `migrate_from`" + assert self.migrate_to is not None, "You must define `migrate_to`" + + # Step 1: Set up the MigrationExecutor + executor = MigrationExecutor(connection) + + # Step 2: Reverse to the starting migration state + migrate_from = [(self.app, self.migrate_from)] + migrate_to = [(self.app, self.migrate_to)] + old_migrate_state = executor.migrate(migrate_from) + old_apps = old_migrate_state.apps + + # Step 3: Run the `setup_before_migration` hook with old state models + self.setup_before_migration(old_apps) + + # Step 4: Apply the migration we want to test with any settings overrides + with override_settings(**self.setting_overrides): + executor = MigrationExecutor(connection) + executor.loader.build_graph() # reload the graph in case of dependency changes + executor.migrate(migrate_to) + + # Set `self.apps` to the post-migration state + self.apps = executor.loader.project_state(migrate_to).apps + + def setup_before_migration(self, apps): + """ + Hook to set up the state before the migration. Subclasses can + override this to create data or perform other setup actions. + """ + pass + + +class TestMigrateAPICredentialToService(BaseMigrationTest): + app = "vng_api_common" + migrate_from = "0005_auto_20190614_1346" + migrate_to = "0006_delete_apicredential" + + def setup_before_migration(self, apps): + Service = apps.get_model("zgw_consumers", "Service") + APICredential = apps.get_model("vng_api_common", "APICredential") + + APICredential.objects.create( + api_root="https://example.com/zaken/api/v1/", + label="Zaken API", + client_id="user", + secret="secret", + user_id="user", + user_representation="User", + ) + + APICredential.objects.create( + api_root="https://example.com/api/v1/", + label="Selectielijst API", + client_id="user2", + secret="secret2", + user_id="user2", + user_representation="User2", + ) + + APICredential.objects.create( + api_root="https://example.com/documenten/api/v1/", + label="Documenten API", + client_id="user3", + secret="secret3", + user_id="user3", + user_representation="User3", + ) + Service.objects.create( + api_root="https://example.com/documenten/api/v1/", + label="Documenten", + slug="already-exists", + api_type=APITypes.drc, + auth_type=AuthTypes.zgw, + client_id="user4", + secret="secret4", + user_id="user4", + user_representation="User4", + ) + + def test_migrate_api_credentials_to_services(self): + Service = global_apps.get_model("zgw_consumers", "Service") + + zaken_service = Service.objects.get(slug="zaken-api") + + assert zaken_service.api_root == "https://example.com/zaken/api/v1/" + assert zaken_service.label == "Zaken API" + assert zaken_service.api_type == APITypes.zrc + assert zaken_service.auth_type == AuthTypes.zgw + assert zaken_service.client_id == "user" + assert zaken_service.secret == "secret" + assert zaken_service.user_id == "user" + assert zaken_service.user_representation == "User" + + selectielijst_service = Service.objects.get(slug="selectielijst-api") + + assert selectielijst_service.api_root == "https://example.com/api/v1/" + assert selectielijst_service.label == "Selectielijst API" + assert selectielijst_service.api_type == APITypes.orc + assert selectielijst_service.auth_type == AuthTypes.zgw + assert selectielijst_service.client_id == "user2" + assert selectielijst_service.secret == "secret2" + assert selectielijst_service.user_id == "user2" + assert selectielijst_service.user_representation == "User2" + + documenten_service = Service.objects.get(slug="already-exists") + + # Because there already was a service for this API root, that data is used instead + assert documenten_service.api_root == "https://example.com/documenten/api/v1/" + assert documenten_service.label == "Documenten" + assert documenten_service.api_type == APITypes.drc + assert documenten_service.auth_type == AuthTypes.zgw + assert documenten_service.client_id == "user4" + assert documenten_service.secret == "secret4" + assert documenten_service.user_id == "user4" + assert documenten_service.user_representation == "User4" + + +class TestMigrateAuthorizationsConfigAPIRootToService(BaseMigrationTest): + app = "authorizations" + migrate_from = "0015_auto_20220318_1608" + migrate_to = "0016_remove_authorizationsconfig_api_root_and_more" + + def setup_before_migration(self, apps): + Service = apps.get_model("zgw_consumers", "Service") + AuthorizationsConfig = apps.get_model("authorizations", "AuthorizationsConfig") + + config, _ = AuthorizationsConfig.objects.get_or_create() + config.api_root = "https://example.com/autorisaties/api/v1/" + config.save() + + self.service = Service.objects.create( + api_root="https://example.com/autorisaties/api/v1/", + label="Autorisaties", + slug="autorisaties", + api_type=APITypes.ac, + auth_type=AuthTypes.zgw, + client_id="user", + secret="secret", + user_id="user", + user_representation="User", + ) + + def test_migrate_api_root_to_service(self): + AuthorizationsConfig = global_apps.get_model( + "authorizations", "AuthorizationsConfig" + ) + + config, _ = AuthorizationsConfig.objects.get_or_create() + + assert config.authorizations_api_service.pk == self.service.pk + assert ( + config.authorizations_api_service.api_root + == "https://example.com/autorisaties/api/v1/" + ) diff --git a/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py b/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py index 1ebde0f3..10aa4c22 100644 --- a/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py +++ b/vng_api_common/authorizations/migrations/0016_remove_authorizationsconfig_api_root_and_more.py @@ -18,7 +18,7 @@ def migrate_authorization_config_to_service(apps, _) -> None: service_label = "Authorization API service" config, _ = AuthorizationsConfig.objects.get_or_create() - _, created = Service.objects.get_or_create( + service, created = Service.objects.get_or_create( api_root=config.api_root, defaults=dict( label="Authorization API service", @@ -27,6 +27,8 @@ def migrate_authorization_config_to_service(apps, _) -> None: auth_type=AuthTypes.zgw, ), ) + config.authorizations_api_service = service + config.save() if created: logger.info(f"Created new Service for {config.api_root}") @@ -38,8 +40,8 @@ def migrate_authorization_config_to_config(apps, _) -> None: AuthorizationsConfig = apps.get_model("authorizations", "AuthorizationsConfig") config, _ = AuthorizationsConfig.objects.get_or_create() - config.api_root = config.service.api_root - + if config.authorizations_api_service: + config.api_root = config.authorizations_api_service.api_root config.save() From 120062f570b35c8a57f4aa3c6525388e04b1dfaa Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Tue, 12 Nov 2024 12:29:18 +0100 Subject: [PATCH 31/38] Add zgw-consumers-oas to requirements Used for the ResourceValidator, which verifies if objects have the correct shape as defined in an OAS. --- setup.cfg | 1 + vng_api_common/tests/schema.py | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/setup.cfg b/setup.cfg index 5426e2d3..12b5a020 100644 --- a/setup.cfg +++ b/setup.cfg @@ -45,6 +45,7 @@ install_requires = isodate notifications-api-common>=0.3.0 zgw-consumers>=0.35.1 + zgw-consumers-oas oyaml PyJWT>=2.0.0 requests diff --git a/vng_api_common/tests/schema.py b/vng_api_common/tests/schema.py index 1794111b..ea143f10 100644 --- a/vng_api_common/tests/schema.py +++ b/vng_api_common/tests/schema.py @@ -4,6 +4,9 @@ from django.conf import settings +from zgw_consumers_oas.schema_loading import read_schema + +from requests_mock import Mocker import yaml DEFAULT_PATH_PARAMETERS = {"version": "1"} @@ -69,3 +72,11 @@ def get_validation_errors(response, field, index=0): return error i += 1 + + +def mock_service_oas_get(mock: Mocker, url: str, service: str, oas_url: str = "") -> None: + if not oas_url: + oas_url = f"{url}schema/openapi.yaml?v=3" + + content = read_schema(service) + mock.get(oas_url, content=content) From 44a40320f9a6f3506bbd1f7a9d2a2c117a528fb5 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Wed, 13 Nov 2024 12:21:14 +0100 Subject: [PATCH 32/38] call `raise_for_status` when testing AC & NRC connections --- tests/test_views.py | 188 ++++++++++++++++++++++++++++++++- vng_api_common/tests/schema.py | 9 +- vng_api_common/views.py | 26 ++--- 3 files changed, 201 insertions(+), 22 deletions(-) diff --git a/tests/test_views.py b/tests/test_views.py index fc343138..cf13dc30 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -1,23 +1,205 @@ -from unittest.mock import patch - +from django.template.response import TemplateResponse from django.urls import reverse import pytest +import requests_mock from notifications_api_common.models import NotificationsConfig +from requests import Response from rest_framework import status +from zgw_consumers.test.factories import ServiceFactory + +from vng_api_common.authorizations.models import AuthorizationsConfig @pytest.mark.django_db def test_config_view(api_client): + notifications_service = ServiceFactory( + api_root="https://notificaties-api.vng.cloud/api/v1/" + ) + notifications_config = NotificationsConfig.get_solo() + notifications_config.notifications_api_service = notifications_service + notifications_config.save() + + authorizations_service = ServiceFactory( + api_root="https://autorisaties-api.vng.cloud/api/v1/", client_id="foobar" + ) + authorizations_config = AuthorizationsConfig.get_solo() + authorizations_config.authorizations_api_service = authorizations_service + authorizations_config.save() + + path = reverse("view-config") + + expected_request_notifications_url = f"{notifications_service.api_root}kanaal" + + expected_request_authorizations_url = ( + f"{authorizations_service.api_root}applicaties?clientIds=foobar" + ) + + with requests_mock.Mocker() as request_mocker: + request_mocker.get(expected_request_notifications_url, status_code=200) + request_mocker.get(expected_request_authorizations_url, status_code=200) + + response = api_client.get(path) + + assert response.status_code == status.HTTP_200_OK + + assert request_mocker.call_count == 2 + + authorization_request = request_mocker.request_history[0] + assert authorization_request.url == expected_request_authorizations_url + + notifications_request = request_mocker.request_history[1] + assert notifications_request.url == expected_request_notifications_url + + +@pytest.mark.django_db +def test_config_view_missing_notifications_service(api_client): """ regression test for https://github.com/open-zaak/open-notificaties/issues/119 """ notifications_config = NotificationsConfig.get_solo() notifications_config.notifications_api_service = None + notifications_config.save() + + authorizations_service = ServiceFactory( + api_root="https://autorisaties-api.vng.cloud/api/v1/", client_id="foobar" + ) + authorizations_config = AuthorizationsConfig.get_solo() + authorizations_config.authorizations_api_service = authorizations_service + authorizations_config.save() + + path = reverse("view-config") + + expected_request_authorizations_url = ( + f"{authorizations_service.api_root}applicaties?clientIds=foobar" + ) + + with requests_mock.Mocker() as request_mocker: + request_mocker.get(expected_request_authorizations_url, status_code=200) + + response = api_client.get(path) + + assert response.status_code == status.HTTP_200_OK + + assert request_mocker.call_count == 1 + authorization_request = request_mocker.request_history[0] + assert authorization_request.url == expected_request_authorizations_url + + +@pytest.mark.django_db +def test_config_view_notifications_error_response(api_client): + notifications_service = ServiceFactory( + api_root="https://notificaties-api.vng.cloud/api/v1/" + ) + notifications_config = NotificationsConfig.get_solo() + notifications_config.notifications_api_service = notifications_service notifications_config.save() + authorizations_service = ServiceFactory( + api_root="https://autorisaties-api.vng.cloud/api/v1/", client_id="foobar" + ) + authorizations_config = AuthorizationsConfig.get_solo() + authorizations_config.authorizations_api_service = authorizations_service + authorizations_config.save() + path = reverse("view-config") - response = api_client.get(path) + + expected_request_notifications_url = f"{notifications_service.api_root}kanaal" + + expected_request_authorizations_url = ( + f"{authorizations_service.api_root}applicaties?clientIds=foobar" + ) + + with requests_mock.Mocker() as request_mocker: + request_mocker.get(expected_request_notifications_url, status_code=403) + request_mocker.get(expected_request_authorizations_url, status_code=200) + + response: TemplateResponse = api_client.get(path) + + response_content = response.content.decode("utf-8") assert response.status_code == status.HTTP_200_OK + assert "Could not connect with NRC" in response_content + + assert request_mocker.call_count == 2 + + authorization_request = request_mocker.request_history[0] + assert authorization_request.url == expected_request_authorizations_url + + notifications_request = request_mocker.request_history[1] + assert notifications_request.url == expected_request_notifications_url + + +@pytest.mark.django_db +def test_config_view_missing_authorizations_service(api_client): + notifications_service = ServiceFactory( + api_root="https://notificaties-api.vng.cloud/api/v1/" + ) + notifications_config = NotificationsConfig.get_solo() + notifications_config.notifications_api_service = notifications_service + notifications_config.save() + + authorizations_config = AuthorizationsConfig.get_solo() + authorizations_config.authorizations_api_service = None + authorizations_config.save() + + path = reverse("view-config") + + expected_request_notifications_url = f"{notifications_service.api_root}kanaal" + + with requests_mock.Mocker() as request_mocker: + request_mocker.get(expected_request_notifications_url, status_code=200) + + response = api_client.get(path) + + assert response.status_code == status.HTTP_200_OK + + assert request_mocker.call_count == 1 + + notifications_request = request_mocker.request_history[0] + assert notifications_request.url == expected_request_notifications_url + + +@pytest.mark.django_db +def test_config_view_authorizations_error_response(api_client): + notifications_service = ServiceFactory( + api_root="https://notificaties-api.vng.cloud/api/v1/" + ) + notifications_config = NotificationsConfig.get_solo() + notifications_config.notifications_api_service = notifications_service + notifications_config.save() + + authorizations_service = ServiceFactory( + api_root="https://autorisaties-api.vng.cloud/api/v1/", client_id="foobar" + ) + authorizations_config = AuthorizationsConfig.get_solo() + authorizations_config.authorizations_api_service = authorizations_service + authorizations_config.save() + + path = reverse("view-config") + + expected_request_notifications_url = f"{notifications_service.api_root}kanaal" + + expected_request_authorizations_url = ( + f"{authorizations_service.api_root}applicaties?clientIds=foobar" + ) + + with requests_mock.Mocker() as request_mocker: + request_mocker.get(expected_request_notifications_url, status_code=200) + request_mocker.get(expected_request_authorizations_url, status_code=403) + + response: TemplateResponse = api_client.get(path) + + response_content = response.content.decode("utf-8") + + assert response.status_code == status.HTTP_200_OK + assert "Could not connect with AC" in response_content + + assert request_mocker.call_count == 2 + + authorization_request = request_mocker.request_history[0] + assert authorization_request.url == expected_request_authorizations_url + + notifications_request = request_mocker.request_history[1] + assert notifications_request.url == expected_request_notifications_url diff --git a/vng_api_common/tests/schema.py b/vng_api_common/tests/schema.py index ea143f10..38138e4f 100644 --- a/vng_api_common/tests/schema.py +++ b/vng_api_common/tests/schema.py @@ -4,10 +4,9 @@ from django.conf import settings -from zgw_consumers_oas.schema_loading import read_schema - -from requests_mock import Mocker import yaml +from requests_mock import Mocker +from zgw_consumers_oas.schema_loading import read_schema DEFAULT_PATH_PARAMETERS = {"version": "1"} @@ -74,7 +73,9 @@ def get_validation_errors(response, field, index=0): i += 1 -def mock_service_oas_get(mock: Mocker, url: str, service: str, oas_url: str = "") -> None: +def mock_service_oas_get( + mock: Mocker, url: str, service: str, oas_url: str = "" +) -> None: if not oas_url: oas_url = f"{url}schema/openapi.yaml?v=3" diff --git a/vng_api_common/views.py b/vng_api_common/views.py index 7d04887c..66b29767 100644 --- a/vng_api_common/views.py +++ b/vng_api_common/views.py @@ -1,6 +1,7 @@ import logging import os from collections import OrderedDict +from typing import Optional from django.apps import apps from django.conf import settings @@ -14,7 +15,7 @@ from rest_framework.response import Response from rest_framework.views import exception_handler as drf_exception_handler -from vng_api_common.client import ClientError +from vng_api_common.client import Client, ClientError from . import exceptions from .compat import sentry_client @@ -129,7 +130,7 @@ def _test_ac_config() -> list: auth_config = AuthorizationsConfig.get_solo() # check if AC auth is configured - ac_client = AuthorizationsConfig.get_client() + ac_client: Optional[Client] = AuthorizationsConfig.get_client() has_ac_auth = ac_client.auth is not None if ac_client else False checks = [ @@ -157,15 +158,14 @@ def _test_ac_config() -> list: client_id = ac_client.auth.service.client_id try: - ac_client.get("applicaties", params={"clientIds": client_id}) + response: requests.Response = ac_client.get( + "applicaties", params={"clientIds": client_id} + ) + + response.raise_for_status() except requests.RequestException: error = True message = _("Could not connect with AC") - except ClientError as exc: - error = True - message = _( - "Cannot retrieve authorizations: HTTP {status_code} - {error_code}" - ).format(status_code=exc.args[0]["status"], error_code=exc.args[0]["code"]) else: message = _("Can retrieve authorizations") @@ -181,7 +181,7 @@ def _test_nrc_config() -> list: from notifications_api_common.models import NotificationsConfig, Subscription nrc_config = NotificationsConfig.get_solo() - nrc_client = NotificationsConfig.get_client() + nrc_client: Optional[Client] = NotificationsConfig.get_client() has_nrc_auth = nrc_client.auth is not None if nrc_client else False @@ -207,15 +207,11 @@ def _test_nrc_config() -> list: error = False try: - nrc_client.get("kanaal") + response: requests.Response = nrc_client.get("kanaal") + response.raise_for_status() except requests.RequestException: error = True message = _("Could not connect with NRC") - except ClientError as exc: - error = True - message = _( - "Cannot retrieve kanalen: HTTP {status_code} - {error_code}" - ).format(status_code=exc.args[0]["status"], error_code=exc.args[0]["code"]) else: message = _("Can retrieve kanalen") From 4517d90b08c58e4059257c0aad47c96fa86efc81 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Thu, 14 Nov 2024 14:35:20 +0100 Subject: [PATCH 33/38] :ok_hand: [maykinmedia/open-api-framework#66] PR feedback --- CHANGELOG.rst | 16 +++++++++++++ README.rst | 3 --- docs/ref/client.rst | 4 ++-- vng_api_common/utils.py | 3 --- vng_api_common/validators.py | 45 ------------------------------------ 5 files changed, 18 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 21c59f4d..8a01537f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,22 @@ Change history ============== +2.0.0 (2024-11-??) +------------------ + +* upgrade to zgw-consumers 0.35.1 +* remove zds-client dependency + +.. warning:: + + The ``APICredential`` class has been removed in favor of the ``Service`` model from zgw-consumers + +.. warning:: + + Several notifications related models (``NotificationsConfig`` and ``Subscription``) as well as + the constants ``SCOPE_NOTIFICATIES_CONSUMEREN_LABEL`` and ``SCOPE_NOTIFICATIES_PUBLICEREN_LABEL`` have + been removed, since they are defined in ``notifications-api-common`` and were a not deleted yet in ``commonground-api-common`` + 1.13.4 (2024-10-25) ------------------- diff --git a/README.rst b/README.rst index 629264b9..f4c47747 100644 --- a/README.rst +++ b/README.rst @@ -35,9 +35,6 @@ Features * ``UniekeIdentificatieValidator`` (in combinatie met organisatie) * ``InformatieObjectUniqueValidator`` om te valideren dat M2M entries slechts eenmalig voorkomen - * ``ObjectInformatieObjectValidator`` om te valideren dat de synchronisatie - van een object-informatieobject relatie pas kan nadat deze relatie in het - DRC gemaakt is * ``IsImmutableValidator`` - valideer dat bepaalde velden niet gewijzigd worden bij een (partial) update, maar wel mogen gezet worden bij een create * ``ResourceValidator`` - valideer dat een URL een bepaalde resource ontsluit diff --git a/docs/ref/client.rst b/docs/ref/client.rst index f3a2dc75..da71db64 100644 --- a/docs/ref/client.rst +++ b/docs/ref/client.rst @@ -5,8 +5,8 @@ Obtaining a client Internally, the `APIClient`_ client is used to resolve remote API objects. To allow the `APIClient`_ to correctly, e.g use correct credentials for authentication , an `Service`_ instance is required with a matching `api_root`. This replaces -the previous mechanism to use `APICredentials` for this purpose. A data migration -will be performed to migrate `APICredentials` to the `Service`_ model. +the previous mechanism to use `APICredentials` for this purpose. A transition from `APICredentials` to +the `Service`_ model is performed automatically with the data migration. .. _APIClient: https://ape-pie.readthedocs.io/en/stable/reference.html#apiclient-class diff --git a/vng_api_common/utils.py b/vng_api_common/utils.py index d6df5c64..0207c1bc 100644 --- a/vng_api_common/utils.py +++ b/vng_api_common/utils.py @@ -12,11 +12,8 @@ from django.utils.encoding import smart_str from django.utils.module_loading import import_string -from requests import RequestException from rest_framework.utils import formatting -from .client import get_client, to_internal_data - try: from djangorestframework_camel_case.util import ( underscore_to_camel as _underscore_to_camel, diff --git a/vng_api_common/validators.py b/vng_api_common/validators.py index 1e3b5034..5bdd74af 100644 --- a/vng_api_common/validators.py +++ b/vng_api_common/validators.py @@ -11,10 +11,8 @@ from django.utils.module_loading import import_string from django.utils.translation import gettext_lazy as _ -import requests from rest_framework import serializers, validators -from .client import get_client, to_internal_data from .constants import RSIN_LENGTH from .oas import fetcher, obj_has_shape @@ -221,49 +219,6 @@ def __call__(self, informatieobject: str, serializer): super().__call__(attrs) -class ObjectInformatieObjectValidator: - """ - Validate that the INFORMATIEOBJECT is linked already in the DRC. - """ - - message = _( - "Het informatieobject is in het DRC nog niet gerelateerd aan dit object." - ) - code = "inconsistent-relation" - requires_context = True - - def __call__(self, informatieobject: str, serializer): - object_url = serializer.context["parent_object"].get_absolute_api_url( - self.request - ) - - # dynamic so that it can be mocked in tests easily - client = get_client(informatieobject) - - if not client: - raise ValidationError( - _("Geen service geconfigureerd voor %s") % informatieobject - ) - - try: - response = client.get( - "objectinformatieobject", - params={ - "informatieobject": informatieobject, - "object": object_url, - }, - ) - - oios = to_internal_data(response) - except (requests.RequestException, RuntimeError) as exc: - raise serializers.ValidationError( - exc.args[0], code="relation-validation-error" - ) from exc - - if len(oios) == 0: - raise serializers.ValidationError(self.message, code=self.code) - - @deconstructible class UntilNowValidator: """ From 5d331a67b25e010b0797239f5a2d18c4f1f030b7 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Thu, 14 Nov 2024 14:55:12 +0100 Subject: [PATCH 34/38] :heavy_plus_sign: Add missing requests-mock dependency --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 12b5a020..512c2ce1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -49,6 +49,7 @@ install_requires = oyaml PyJWT>=2.0.0 requests + requests-mock coreapi ape-pie tests_require = From df707636b4d1217d7f3583a852befffd69de9a5e Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Fri, 15 Nov 2024 16:44:15 +0100 Subject: [PATCH 35/38] Handle duplicate service slugs --- tests/test_migrations.py | 231 +++++++++++------- .../migrations/0006_delete_apicredential.py | 63 +++-- 2 files changed, 182 insertions(+), 112 deletions(-) diff --git a/tests/test_migrations.py b/tests/test_migrations.py index 5323f1d3..f2c59ee6 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -1,4 +1,7 @@ +from typing import Callable, Generator + from django.apps import apps as global_apps +from django.apps.registry import Apps from django.db import connection from django.db.migrations.executor import MigrationExecutor from django.test import override_settings @@ -13,43 +16,36 @@ class BaseMigrationTest: migrate_to = None # The migration we want to test setting_overrides = {} - @pytest.fixture(autouse=True) - def setup_migration(self, db): + @pytest.fixture + def setup_migration(self, db) -> Generator: """ Setup the migration test by reversing to `migrate_from` state, then applying the `migrate_to` state. """ - assert self.app is not None, "You must define the `app` attribute" - assert self.migrate_from is not None, "You must define `migrate_from`" - assert self.migrate_to is not None, "You must define `migrate_to`" - # Step 1: Set up the MigrationExecutor - executor = MigrationExecutor(connection) + def _migrate(callback: Callable) -> None: + assert self.app is not None, "You must define the `app` attribute" + assert self.migrate_from is not None, "You must define `migrate_from`" + assert self.migrate_to is not None, "You must define `migrate_to`" + + # Step 1: Set up the MigrationExecutor + executor = MigrationExecutor(connection) - # Step 2: Reverse to the starting migration state - migrate_from = [(self.app, self.migrate_from)] - migrate_to = [(self.app, self.migrate_to)] - old_migrate_state = executor.migrate(migrate_from) - old_apps = old_migrate_state.apps + # Step 2: Reverse to the starting migration state + migrate_from = [(self.app, self.migrate_from)] + migrate_to = [(self.app, self.migrate_to)] + old_migrate_state = executor.migrate(migrate_from) + old_apps = old_migrate_state.apps - # Step 3: Run the `setup_before_migration` hook with old state models - self.setup_before_migration(old_apps) + # Step 3: Call `callback` old state models + callback(old_apps) - # Step 4: Apply the migration we want to test with any settings overrides - with override_settings(**self.setting_overrides): + # Step 4: Apply the migration we want to test with any settings overrides executor = MigrationExecutor(connection) executor.loader.build_graph() # reload the graph in case of dependency changes executor.migrate(migrate_to) - # Set `self.apps` to the post-migration state - self.apps = executor.loader.project_state(migrate_to).apps - - def setup_before_migration(self, apps): - """ - Hook to set up the state before the migration. Subclasses can - override this to create data or perform other setup actions. - """ - pass + yield _migrate class TestMigrateAPICredentialToService(BaseMigrationTest): @@ -57,49 +53,51 @@ class TestMigrateAPICredentialToService(BaseMigrationTest): migrate_from = "0005_auto_20190614_1346" migrate_to = "0006_delete_apicredential" - def setup_before_migration(self, apps): - Service = apps.get_model("zgw_consumers", "Service") - APICredential = apps.get_model("vng_api_common", "APICredential") - - APICredential.objects.create( - api_root="https://example.com/zaken/api/v1/", - label="Zaken API", - client_id="user", - secret="secret", - user_id="user", - user_representation="User", - ) - - APICredential.objects.create( - api_root="https://example.com/api/v1/", - label="Selectielijst API", - client_id="user2", - secret="secret2", - user_id="user2", - user_representation="User2", - ) - - APICredential.objects.create( - api_root="https://example.com/documenten/api/v1/", - label="Documenten API", - client_id="user3", - secret="secret3", - user_id="user3", - user_representation="User3", - ) - Service.objects.create( - api_root="https://example.com/documenten/api/v1/", - label="Documenten", - slug="already-exists", - api_type=APITypes.drc, - auth_type=AuthTypes.zgw, - client_id="user4", - secret="secret4", - user_id="user4", - user_representation="User4", - ) + def test_migrate_api_credentials_to_services(self, setup_migration: Callable): + def migration_callback(apps: Apps) -> None: + Service = apps.get_model("zgw_consumers", "Service") + APICredential = apps.get_model("vng_api_common", "APICredential") + + APICredential.objects.create( + api_root="https://example.com/zaken/api/v1/", + label="Zaken API", + client_id="user", + secret="secret", + user_id="user", + user_representation="User", + ) + + APICredential.objects.create( + api_root="https://example.com/api/v1/", + label="Selectielijst API", + client_id="user2", + secret="secret2", + user_id="user2", + user_representation="User2", + ) + + APICredential.objects.create( + api_root="https://example.com/documenten/api/v1/", + label="Documenten API", + client_id="user3", + secret="secret3", + user_id="user3", + user_representation="User3", + ) + Service.objects.create( + api_root="https://example.com/documenten/api/v1/", + label="Documenten", + slug="already-exists", + api_type=APITypes.drc, + auth_type=AuthTypes.zgw, + client_id="user4", + secret="secret4", + user_id="user4", + user_representation="User4", + ) + + setup_migration(migration_callback) - def test_migrate_api_credentials_to_services(self): Service = global_apps.get_model("zgw_consumers", "Service") zaken_service = Service.objects.get(slug="zaken-api") @@ -136,33 +134,92 @@ def test_migrate_api_credentials_to_services(self): assert documenten_service.user_id == "user4" assert documenten_service.user_representation == "User4" + def test_duplicate_service_slug(self, setup_migration): + """ + Test that the migration handles duplicate slugs correctly + """ + def migration_callback(apps): + Service = apps.get_model("zgw_consumers", "Service") + APICredential = apps.get_model("vng_api_common", "APICredential") + + APICredential.objects.create( + api_root="https://example.com/documenten/api/v1/", + label="Documenten API", + client_id="user3", + secret="secret3", + user_id="user3", + user_representation="User3", + ) + + Service.objects.create( + api_root="https://example.com/documenten/api/v2/", + label="Documenten", + slug="documenten-api", + api_type=APITypes.drc, + auth_type=AuthTypes.zgw, + client_id="user4", + secret="secret4", + user_id="user4", + user_representation="User4", + ) + + setup_migration(migration_callback) + + Service = global_apps.get_model("zgw_consumers", "Service") + + service_v1 = Service.objects.get(slug="documenten-api-2") + + assert service_v1.api_root == "https://example.com/documenten/api/v1/" + assert service_v1.label == "Documenten API" + assert service_v1.api_type == APITypes.drc + assert service_v1.auth_type == AuthTypes.zgw + assert service_v1.client_id == "user3" + assert service_v1.secret == "secret3" + assert service_v1.user_id == "user3" + assert service_v1.user_representation == "User3" + + service_v2 = Service.objects.get(slug="documenten-api") + + assert service_v2.api_root == "https://example.com/documenten/api/v2/" + assert service_v2.label == "Documenten" + assert service_v2.api_type == APITypes.drc + assert service_v2.auth_type == AuthTypes.zgw + assert service_v2.client_id == "user4" + assert service_v2.secret == "secret4" + assert service_v2.user_id == "user4" + assert service_v2.user_representation == "User4" + class TestMigrateAuthorizationsConfigAPIRootToService(BaseMigrationTest): app = "authorizations" migrate_from = "0015_auto_20220318_1608" migrate_to = "0016_remove_authorizationsconfig_api_root_and_more" - def setup_before_migration(self, apps): - Service = apps.get_model("zgw_consumers", "Service") - AuthorizationsConfig = apps.get_model("authorizations", "AuthorizationsConfig") - - config, _ = AuthorizationsConfig.objects.get_or_create() - config.api_root = "https://example.com/autorisaties/api/v1/" - config.save() - - self.service = Service.objects.create( - api_root="https://example.com/autorisaties/api/v1/", - label="Autorisaties", - slug="autorisaties", - api_type=APITypes.ac, - auth_type=AuthTypes.zgw, - client_id="user", - secret="secret", - user_id="user", - user_representation="User", - ) + def test_migrate_api_root_to_service(self, setup_migration: Callable): + def migration_callback(apps: Apps) -> None: + Service = apps.get_model("zgw_consumers", "Service") + AuthorizationsConfig = apps.get_model( + "authorizations", "AuthorizationsConfig" + ) + + config, _ = AuthorizationsConfig.objects.get_or_create() + config.api_root = "https://example.com/autorisaties/api/v1/" + config.save() + + self.service = Service.objects.create( + api_root="https://example.com/autorisaties/api/v1/", + label="Autorisaties", + slug="autorisaties", + api_type=APITypes.ac, + auth_type=AuthTypes.zgw, + client_id="user", + secret="secret", + user_id="user", + user_representation="User", + ) + + setup_migration(migration_callback) - def test_migrate_api_root_to_service(self): AuthorizationsConfig = global_apps.get_model( "authorizations", "AuthorizationsConfig" ) diff --git a/vng_api_common/migrations/0006_delete_apicredential.py b/vng_api_common/migrations/0006_delete_apicredential.py index 5318e50b..a366f376 100644 --- a/vng_api_common/migrations/0006_delete_apicredential.py +++ b/vng_api_common/migrations/0006_delete_apicredential.py @@ -1,8 +1,9 @@ # Generated by Django 5.1.2 on 2024-10-24 13:51 import logging +from typing import Set -from django.db import IntegrityError, migrations +from django.db import migrations, models from django.utils.text import slugify from zgw_consumers.constants import APITypes, AuthTypes @@ -10,7 +11,7 @@ logger = logging.getLogger(__name__) -def get_api_type(api_root: str) -> APITypes: +def _get_api_type(api_root: str) -> APITypes: mapping = { "/autorisaties/api/": APITypes.ac, "/zaken/api/": APITypes.zrc, @@ -26,38 +27,50 @@ def get_api_type(api_root: str) -> APITypes: return APITypes.orc +def _get_service_slug(credential: models.Model, existing_slugs: Set[str]) -> str: + default_slug: str = slugify(credential.label) + + if default_slug not in existing_slugs or not existing_slugs: + return default_slug + + count = 2 + slug = f"{default_slug}-{count}" + + while slug in existing_slugs: + count += 1 + slug = f"{default_slug}-{count}" + + return slug + + def migrate_credentials_to_service(apps, _) -> None: APICredential = apps.get_model("vng_api_common", "APICredential") Service = apps.get_model("zgw_consumers", "Service") credentials = APICredential.objects.all() + existings_service_slugs = set(Service.objects.values_list("slug", flat=True)) + for credential in credentials: logger.info(f"Creating Service for {credential.client_id}") - service_slug = slugify(credential.label) - - try: - _, created = Service.objects.get_or_create( - api_root=credential.api_root, - defaults=dict( - label=credential.label, - slug=service_slug, - api_type=get_api_type(credential.api_root), - auth_type=AuthTypes.zgw, - client_id=credential.client_id, - secret=credential.secret, - user_id=credential.user_id, - user_representation=credential.user_representation, - ), - ) - except IntegrityError: - logger.warning( - f"Unable to create Service for {credential.api_root}. Check the" - "`service slug` field on the existing Service's to verify an existing" - " Service already exists." - ) - continue + service_slug = _get_service_slug(credential, existings_service_slugs) + + _, created = Service.objects.get_or_create( + api_root=credential.api_root, + defaults=dict( + label=credential.label, + slug=service_slug, + api_type=_get_api_type(credential.api_root), + auth_type=AuthTypes.zgw, + client_id=credential.client_id, + secret=credential.secret, + user_id=credential.user_id, + user_representation=credential.user_representation, + ), + ) + + existings_service_slugs.add(service_slug) if created: logger.info(f"Created new Service for {credential.api_root}") From d83ca95be3ac8be48af5b60f547b26a1340ba8cf Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Mon, 18 Nov 2024 10:47:57 +0100 Subject: [PATCH 36/38] Apply formatting --- tests/test_migrations.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_migrations.py b/tests/test_migrations.py index f2c59ee6..ec3aa7d6 100644 --- a/tests/test_migrations.py +++ b/tests/test_migrations.py @@ -138,6 +138,7 @@ def test_duplicate_service_slug(self, setup_migration): """ Test that the migration handles duplicate slugs correctly """ + def migration_callback(apps): Service = apps.get_model("zgw_consumers", "Service") APICredential = apps.get_model("vng_api_common", "APICredential") From b420445442379a51c5aa35bd5de70d2c8393115d Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Tue, 19 Nov 2024 10:32:07 +0100 Subject: [PATCH 37/38] Move `zgw-consumers-oas` to test requirements --- setup.cfg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 512c2ce1..8b9476e5 100644 --- a/setup.cfg +++ b/setup.cfg @@ -45,7 +45,6 @@ install_requires = isodate notifications-api-common>=0.3.0 zgw-consumers>=0.35.1 - zgw-consumers-oas oyaml PyJWT>=2.0.0 requests @@ -61,6 +60,7 @@ tests_require = black requests-mock freezegun + zgw-consumers-oas [options.packages.find] include = @@ -80,6 +80,7 @@ tests = black requests-mock freezegun + zgw-consumers-oas pep8 = flake8 coverage = pytest-cov docs = From d73f24c0e041371979e5ea0f07ec4d6e9e6ab264 Mon Sep 17 00:00:00 2001 From: Steven Bal Date: Fri, 22 Nov 2024 11:17:12 +0100 Subject: [PATCH 38/38] :bug: Check for correct installed notifs app in nrc config check this checked for vng_api_common.notifications, but should check notifications_api_common since the notifications code moved there --- vng_api_common/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vng_api_common/views.py b/vng_api_common/views.py index 66b29767..1c242677 100644 --- a/vng_api_common/views.py +++ b/vng_api_common/views.py @@ -175,7 +175,7 @@ def _test_ac_config() -> list: def _test_nrc_config() -> list: - if not apps.is_installed("vng_api_common.notifications"): + if not apps.is_installed("notifications_api_common"): return [] from notifications_api_common.models import NotificationsConfig, Subscription