From 8e4af3bd98fd520be2fadae3d3ea3cbbdb7df4fb Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 21 Jun 2024 11:54:45 +0200 Subject: [PATCH 01/11] :recycle: Rename abstract base model The names of the base classes from the mozilla-django-oidc-db and digid_eherkenning.oidc packages were too confusing. --- CHANGELOG.rst | 7 +++++++ digid_eherkenning/oidc/admin.py | 4 ++-- digid_eherkenning/oidc/backends.py | 4 ++-- digid_eherkenning/oidc/models/__init__.py | 8 ++------ digid_eherkenning/oidc/models/base.py | 2 +- digid_eherkenning/oidc/models/digid.py | 6 +++--- digid_eherkenning/oidc/models/eherkenning.py | 10 +++------- tests/oidc/test_oidc_integration.py | 4 ++-- 8 files changed, 22 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1080c8a..b810091 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,13 @@ Changelog ========= +0.15.0 (2024-06-??) +=================== + +Further iteration on the OIDC integration. + +* 💥⚠️ Renamed the ``OpenIDConnectBaseConfig`` base model to ``BaseConfig`` + 0.14.0 (2024-06-13) =================== diff --git a/digid_eherkenning/oidc/admin.py b/digid_eherkenning/oidc/admin.py index d6e59fa..63b6d84 100644 --- a/digid_eherkenning/oidc/admin.py +++ b/digid_eherkenning/oidc/admin.py @@ -9,11 +9,11 @@ from solo.admin import SingletonModelAdmin from .models import ( + BaseConfig, DigiDConfig, DigiDMachtigenConfig, EHerkenningBewindvoeringConfig, EHerkenningConfig, - OpenIDConnectBaseConfig, ) # Using a dict because these retain ordering, and it makes things a bit more readable. @@ -62,7 +62,7 @@ } -def admin_modelform_factory(model: type[OpenIDConnectBaseConfig], *args, **kwargs): +def admin_modelform_factory(model: type[BaseConfig], *args, **kwargs): """ Factory function to generate a model form class for a given configuration model. diff --git a/digid_eherkenning/oidc/backends.py b/digid_eherkenning/oidc/backends.py index e9ba725..51d69a3 100644 --- a/digid_eherkenning/oidc/backends.py +++ b/digid_eherkenning/oidc/backends.py @@ -3,12 +3,12 @@ from mozilla_django_oidc_db.backends import OIDCAuthenticationBackend from mozilla_django_oidc_db.typing import JSONObject -from .models.base import OpenIDConnectBaseConfig +from .models.base import BaseConfig class BaseBackend(OIDCAuthenticationBackend): def _check_candidate_backend(self) -> bool: - suitable_model = issubclass(self.config_class, OpenIDConnectBaseConfig) + suitable_model = issubclass(self.config_class, BaseConfig) return suitable_model and super()._check_candidate_backend() def update_user(self, user: AbstractUser, claims: JSONObject): diff --git a/digid_eherkenning/oidc/models/__init__.py b/digid_eherkenning/oidc/models/__init__.py index e19c5fb..c644955 100644 --- a/digid_eherkenning/oidc/models/__init__.py +++ b/digid_eherkenning/oidc/models/__init__.py @@ -1,15 +1,11 @@ -from .base import ( - OpenIDConnectBaseConfig, - get_default_scopes_bsn, - get_default_scopes_kvk, -) +from .base import BaseConfig, get_default_scopes_bsn, get_default_scopes_kvk from .digid import DigiDConfig, DigiDMachtigenConfig from .eherkenning import EHerkenningBewindvoeringConfig, EHerkenningConfig __all__ = [ "get_default_scopes_bsn", "get_default_scopes_kvk", - "OpenIDConnectBaseConfig", + "BaseConfig", "DigiDConfig", "DigiDMachtigenConfig", "EHerkenningConfig", diff --git a/digid_eherkenning/oidc/models/base.py b/digid_eherkenning/oidc/models/base.py index ec9c3ad..6058239 100644 --- a/digid_eherkenning/oidc/models/base.py +++ b/digid_eherkenning/oidc/models/base.py @@ -20,7 +20,7 @@ def get_default_scopes_kvk(): return ["openid", "kvk"] -class OpenIDConnectBaseConfig(OpenIDConnectConfigBase): +class BaseConfig(OpenIDConnectConfigBase): """ Base configuration for DigiD/eHerkenning authentication via OpenID Connect. """ diff --git a/digid_eherkenning/oidc/models/digid.py b/digid_eherkenning/oidc/models/digid.py index b7b676e..2b4ece6 100644 --- a/digid_eherkenning/oidc/models/digid.py +++ b/digid_eherkenning/oidc/models/digid.py @@ -7,10 +7,10 @@ from mozilla_django_oidc_db.fields import ClaimField, ClaimFieldDefault from mozilla_django_oidc_db.typing import ClaimPath -from .base import OpenIDConnectBaseConfig, get_default_scopes_bsn +from .base import BaseConfig, get_default_scopes_bsn -class DigiDConfig(OpenIDConnectBaseConfig): +class DigiDConfig(BaseConfig): """ Configuration for DigiD authentication via OpenID connect """ @@ -39,7 +39,7 @@ def oidcdb_username_claim(self) -> ClaimPath: return self.bsn_claim -class DigiDMachtigenConfig(OpenIDConnectBaseConfig): +class DigiDMachtigenConfig(BaseConfig): # TODO: these default claim names don't appear to be part of any standard... representee_bsn_claim = ClaimField( verbose_name=_("representee bsn claim"), diff --git a/digid_eherkenning/oidc/models/eherkenning.py b/digid_eherkenning/oidc/models/eherkenning.py index 2dd2c15..cf18613 100644 --- a/digid_eherkenning/oidc/models/eherkenning.py +++ b/digid_eherkenning/oidc/models/eherkenning.py @@ -7,11 +7,7 @@ from mozilla_django_oidc_db.fields import ClaimField, ClaimFieldDefault from mozilla_django_oidc_db.typing import ClaimPath -from .base import ( - OpenIDConnectBaseConfig, - get_default_scopes_bsn, - get_default_scopes_kvk, -) +from .base import BaseConfig, get_default_scopes_bsn, get_default_scopes_kvk class AuthorizeeMixin(models.Model): @@ -62,7 +58,7 @@ def oidcdb_sensitive_claims(self) -> Sequence[ClaimPath]: ] -class EHerkenningConfig(AuthorizeeMixin, OpenIDConnectBaseConfig): +class EHerkenningConfig(AuthorizeeMixin, BaseConfig): """ Configuration for eHerkenning authentication via OpenID connect. """ @@ -86,7 +82,7 @@ def oidcdb_username_claim(self) -> ClaimPath: return self.legal_subject_claim -class EHerkenningBewindvoeringConfig(AuthorizeeMixin, OpenIDConnectBaseConfig): +class EHerkenningBewindvoeringConfig(AuthorizeeMixin, BaseConfig): # NOTE: Discussion with an employee from Anoigo states this will always be a BSN, # not an RSIN or CoC number. representee_claim = ClaimField( diff --git a/tests/oidc/test_oidc_integration.py b/tests/oidc/test_oidc_integration.py index ae63382..685da9e 100644 --- a/tests/oidc/test_oidc_integration.py +++ b/tests/oidc/test_oidc_integration.py @@ -13,7 +13,7 @@ EHerkenningBewindvoeringConfig, EHerkenningConfig, ) -from digid_eherkenning.oidc.models.base import OpenIDConnectBaseConfig +from digid_eherkenning.oidc.models.base import BaseConfig from digid_eherkenning.oidc.views import ( digid_init, digid_machtigen_init, @@ -44,7 +44,7 @@ def test_init_flow( auth_request: HttpRequest, mocked_responses: RequestsMock, init_view, - config_class: type[OpenIDConnectBaseConfig], + config_class: type[BaseConfig], ): _name = config_class._meta.model_name config = config_class( From 568b7308a302c6b95bc11d04580f2fcbb8c36519 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 21 Jun 2024 12:19:05 +0200 Subject: [PATCH 02/11] :card_file_box: Add configuration fields for LOA * Added claim field to configure claim holding the value for LOA * Added field for LOA fallback value, populated from the respective choices (known identifiers from the standards). --- digid_eherkenning/oidc/admin.py | 14 +- ...ault_loa_digidconfig_loa_claim_and_more.py | 168 ++++++++++++++++++ digid_eherkenning/oidc/models/base.py | 32 ++++ digid_eherkenning/oidc/models/digid.py | 5 +- digid_eherkenning/oidc/models/eherkenning.py | 10 +- 5 files changed, 226 insertions(+), 3 deletions(-) create mode 100644 digid_eherkenning/oidc/migrations/0007_digidconfig_default_loa_digidconfig_loa_claim_and_more.py diff --git a/digid_eherkenning/oidc/admin.py b/digid_eherkenning/oidc/admin.py index 63b6d84..de6327d 100644 --- a/digid_eherkenning/oidc/admin.py +++ b/digid_eherkenning/oidc/admin.py @@ -92,7 +92,13 @@ def fieldsets_factory(claim_mapping_fields: Sequence[str]): @admin.register(DigiDConfig) class DigiDConfigAdmin(SingletonModelAdmin): form = admin_modelform_factory(DigiDConfig) - fieldsets = fieldsets_factory(claim_mapping_fields=["bsn_claim"]) + fieldsets = fieldsets_factory( + claim_mapping_fields=[ + "bsn_claim", + "loa_claim", + "default_loa", + ] + ) @admin.register(EHerkenningConfig) @@ -104,6 +110,8 @@ class EHerkenningConfigAdmin(SingletonModelAdmin): "legal_subject_claim", "branch_number_claim", "acting_subject_claim", + "loa_claim", + "default_loa", ] ) @@ -115,6 +123,8 @@ class DigiDMachtigenConfigAdmin(SingletonModelAdmin): claim_mapping_fields=[ "representee_bsn_claim", "authorizee_bsn_claim", + "loa_claim", + "default_loa", "mandate_service_id_claim", ] ) @@ -130,6 +140,8 @@ class EHerkenningBewindvoeringConfigAdmin(SingletonModelAdmin): "legal_subject_claim", "branch_number_claim", "acting_subject_claim", + "loa_claim", + "default_loa", "mandate_service_id_claim", "mandate_service_uuid_claim", ] diff --git a/digid_eherkenning/oidc/migrations/0007_digidconfig_default_loa_digidconfig_loa_claim_and_more.py b/digid_eherkenning/oidc/migrations/0007_digidconfig_default_loa_digidconfig_loa_claim_and_more.py new file mode 100644 index 0000000..1432456 --- /dev/null +++ b/digid_eherkenning/oidc/migrations/0007_digidconfig_default_loa_digidconfig_loa_claim_and_more.py @@ -0,0 +1,168 @@ +# Generated by Django 4.2.13 on 2024-06-21 10:17 + +from django.db import migrations, models + +import mozilla_django_oidc_db.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "digid_eherkenning_oidc_generics", + "0006_alter_digidconfig_oidc_rp_scopes_list_and_more", + ), + ] + + operations = [ + migrations.AddField( + model_name="digidconfig", + name="default_loa", + field=models.CharField( + blank=True, + choices=[ + ( + "urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport", + "DigiD Basis", + ), + ( + "urn:oasis:names:tc:SAML:2.0:ac:classes:MobileTwoFactorContract", + "DigiD Midden", + ), + ( + "urn:oasis:names:tc:SAML:2.0:ac:classes:Smartcard", + "DigiD Substantieel", + ), + ( + "urn:oasis:names:tc:SAML:2.0:ac:classes:SmartcardPKI", + "DigiD Hoog", + ), + ], + help_text="Fallback level of assurance, in case no claim value could be extracted.", + max_length=100, + verbose_name="default LOA", + ), + ), + migrations.AddField( + model_name="digidconfig", + name="loa_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + blank=True, + default=None, + help_text="Name of the claim holding the level of assurance. If left empty, it is assumed there is no LOA claim and the configured callback value will be used.", + null=True, + size=None, + verbose_name="LoA claim", + ), + ), + migrations.AddField( + model_name="digidmachtigenconfig", + name="default_loa", + field=models.CharField( + blank=True, + choices=[ + ( + "urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport", + "DigiD Basis", + ), + ( + "urn:oasis:names:tc:SAML:2.0:ac:classes:MobileTwoFactorContract", + "DigiD Midden", + ), + ( + "urn:oasis:names:tc:SAML:2.0:ac:classes:Smartcard", + "DigiD Substantieel", + ), + ( + "urn:oasis:names:tc:SAML:2.0:ac:classes:SmartcardPKI", + "DigiD Hoog", + ), + ], + help_text="Fallback level of assurance, in case no claim value could be extracted.", + max_length=100, + verbose_name="default LOA", + ), + ), + migrations.AddField( + model_name="digidmachtigenconfig", + name="loa_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + blank=True, + default=None, + help_text="Name of the claim holding the level of assurance. If left empty, it is assumed there is no LOA claim and the configured callback value will be used.", + null=True, + size=None, + verbose_name="LoA claim", + ), + ), + migrations.AddField( + model_name="eherkenningbewindvoeringconfig", + name="default_loa", + field=models.CharField( + blank=True, + choices=[ + ("urn:etoegang:core:assurance-class:loa1", "Non existent (1)"), + ("urn:etoegang:core:assurance-class:loa2", "Low (2)"), + ("urn:etoegang:core:assurance-class:loa2plus", "Low (2+)"), + ("urn:etoegang:core:assurance-class:loa3", "Substantial (3)"), + ("urn:etoegang:core:assurance-class:loa4", "High (4)"), + ], + help_text="Fallback level of assurance, in case no claim value could be extracted.", + max_length=100, + verbose_name="default LOA", + ), + ), + migrations.AddField( + model_name="eherkenningbewindvoeringconfig", + name="loa_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + blank=True, + default=None, + help_text="Name of the claim holding the level of assurance. If left empty, it is assumed there is no LOA claim and the configured callback value will be used.", + null=True, + size=None, + verbose_name="LoA claim", + ), + ), + migrations.AddField( + model_name="eherkenningconfig", + name="default_loa", + field=models.CharField( + blank=True, + choices=[ + ("urn:etoegang:core:assurance-class:loa1", "Non existent (1)"), + ("urn:etoegang:core:assurance-class:loa2", "Low (2)"), + ("urn:etoegang:core:assurance-class:loa2plus", "Low (2+)"), + ("urn:etoegang:core:assurance-class:loa3", "Substantial (3)"), + ("urn:etoegang:core:assurance-class:loa4", "High (4)"), + ], + help_text="Fallback level of assurance, in case no claim value could be extracted.", + max_length=100, + verbose_name="default LOA", + ), + ), + migrations.AddField( + model_name="eherkenningconfig", + name="loa_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + blank=True, + default=None, + help_text="Name of the claim holding the level of assurance. If left empty, it is assumed there is no LOA claim and the configured callback value will be used.", + null=True, + size=None, + verbose_name="LoA claim", + ), + ), + ] diff --git a/digid_eherkenning/oidc/models/base.py b/digid_eherkenning/oidc/models/base.py index 6058239..0eb2914 100644 --- a/digid_eherkenning/oidc/models/base.py +++ b/digid_eherkenning/oidc/models/base.py @@ -1,8 +1,10 @@ from django.conf import settings +from django.db import models from django.utils.functional import classproperty from django.utils.module_loading import import_string from django.utils.translation import gettext_lazy as _ +from mozilla_django_oidc_db.fields import ClaimField from mozilla_django_oidc_db.models import OpenIDConnectConfigBase @@ -20,11 +22,41 @@ def get_default_scopes_kvk(): return ["openid", "kvk"] +def default_loa_choices(choicesCls: type[models.TextChoices]): + def decorator(cls: type[models.Model]): + field = cls._meta.get_field("default_loa") + field.choices = choicesCls.choices + return cls + + return decorator + + class BaseConfig(OpenIDConnectConfigBase): """ Base configuration for DigiD/eHerkenning authentication via OpenID Connect. """ + loa_claim = ClaimField( + verbose_name=_("LoA claim"), + default=None, + help_text=_( + "Name of the claim holding the level of assurance. If left empty, it is " + "assumed there is no LOA claim and the configured callback value will be " + "used." + ), + null=True, + blank=True, + ) + default_loa = models.CharField( + _("default LOA"), + max_length=100, + blank=True, + choices=tuple(), # set dynamically via the default_loa_choices decorator + help_text=_( + "Fallback level of assurance, in case no claim value could be extracted." + ), + ) + class Meta: abstract = True diff --git a/digid_eherkenning/oidc/models/digid.py b/digid_eherkenning/oidc/models/digid.py index 2b4ece6..5f6f99b 100644 --- a/digid_eherkenning/oidc/models/digid.py +++ b/digid_eherkenning/oidc/models/digid.py @@ -7,9 +7,11 @@ from mozilla_django_oidc_db.fields import ClaimField, ClaimFieldDefault from mozilla_django_oidc_db.typing import ClaimPath -from .base import BaseConfig, get_default_scopes_bsn +from ...choices import DigiDAssuranceLevels +from .base import BaseConfig, default_loa_choices, get_default_scopes_bsn +@default_loa_choices(DigiDAssuranceLevels) class DigiDConfig(BaseConfig): """ Configuration for DigiD authentication via OpenID connect @@ -39,6 +41,7 @@ def oidcdb_username_claim(self) -> ClaimPath: return self.bsn_claim +@default_loa_choices(DigiDAssuranceLevels) class DigiDMachtigenConfig(BaseConfig): # TODO: these default claim names don't appear to be part of any standard... representee_bsn_claim = ClaimField( diff --git a/digid_eherkenning/oidc/models/eherkenning.py b/digid_eherkenning/oidc/models/eherkenning.py index cf18613..142619b 100644 --- a/digid_eherkenning/oidc/models/eherkenning.py +++ b/digid_eherkenning/oidc/models/eherkenning.py @@ -7,7 +7,13 @@ from mozilla_django_oidc_db.fields import ClaimField, ClaimFieldDefault from mozilla_django_oidc_db.typing import ClaimPath -from .base import BaseConfig, get_default_scopes_bsn, get_default_scopes_kvk +from ...choices import AssuranceLevels +from .base import ( + BaseConfig, + default_loa_choices, + get_default_scopes_bsn, + get_default_scopes_kvk, +) class AuthorizeeMixin(models.Model): @@ -58,6 +64,7 @@ def oidcdb_sensitive_claims(self) -> Sequence[ClaimPath]: ] +@default_loa_choices(AssuranceLevels) class EHerkenningConfig(AuthorizeeMixin, BaseConfig): """ Configuration for eHerkenning authentication via OpenID connect. @@ -82,6 +89,7 @@ def oidcdb_username_claim(self) -> ClaimPath: return self.legal_subject_claim +@default_loa_choices(AssuranceLevels) class EHerkenningBewindvoeringConfig(AuthorizeeMixin, BaseConfig): # NOTE: Discussion with an employee from Anoigo states this will always be a BSN, # not an RSIN or CoC number. From bf4124233bd6e9aea654689a4eb710d805e84323 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 21 Jun 2024 12:54:29 +0200 Subject: [PATCH 03/11] :card_file_box: Add LOA mapping configuration field Different brokers (Anoigo, Signicat) seem to return the LOA from the used authentication mean in a non-standard way, so our configuration needs to be able to translate their values into standardized values. Brokers that don't return a value at all can be configured to specify a default value to apply. Source values (from the broker) can be string or number, destination values are enum-based depending on whether it's DigiD or eHerkenning. --- digid_eherkenning/oidc/admin.py | 6 +- ..._digidconfig_loa_value_mapping_and_more.py | 58 ++++++++++++++++++ digid_eherkenning/oidc/models/base.py | 60 ++++++++++++++++++- 3 files changed, 120 insertions(+), 4 deletions(-) create mode 100644 digid_eherkenning/oidc/migrations/0008_digidconfig_loa_value_mapping_and_more.py diff --git a/digid_eherkenning/oidc/admin.py b/digid_eherkenning/oidc/admin.py index de6327d..d26462c 100644 --- a/digid_eherkenning/oidc/admin.py +++ b/digid_eherkenning/oidc/admin.py @@ -80,7 +80,7 @@ def admin_modelform_factory(model: type[BaseConfig], *args, **kwargs): return Form -def fieldsets_factory(claim_mapping_fields: Sequence[str]): +def fieldsets_factory(claim_mapping_fields: Sequence[str | Sequence[str]]): """ Apply the shared fieldsets configuration with the model-specific overrides. """ @@ -97,6 +97,7 @@ class DigiDConfigAdmin(SingletonModelAdmin): "bsn_claim", "loa_claim", "default_loa", + "loa_value_mapping", ] ) @@ -112,6 +113,7 @@ class EHerkenningConfigAdmin(SingletonModelAdmin): "acting_subject_claim", "loa_claim", "default_loa", + "loa_value_mapping", ] ) @@ -125,6 +127,7 @@ class DigiDMachtigenConfigAdmin(SingletonModelAdmin): "authorizee_bsn_claim", "loa_claim", "default_loa", + "loa_value_mapping", "mandate_service_id_claim", ] ) @@ -142,6 +145,7 @@ class EHerkenningBewindvoeringConfigAdmin(SingletonModelAdmin): "acting_subject_claim", "loa_claim", "default_loa", + "loa_value_mapping", "mandate_service_id_claim", "mandate_service_uuid_claim", ] diff --git a/digid_eherkenning/oidc/migrations/0008_digidconfig_loa_value_mapping_and_more.py b/digid_eherkenning/oidc/migrations/0008_digidconfig_loa_value_mapping_and_more.py new file mode 100644 index 0000000..745ec1c --- /dev/null +++ b/digid_eherkenning/oidc/migrations/0008_digidconfig_loa_value_mapping_and_more.py @@ -0,0 +1,58 @@ +# Generated by Django 4.2.13 on 2024-06-21 10:36 + +from django.db import migrations + +import django_jsonform.models.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "digid_eherkenning_oidc_generics", + "0007_digidconfig_default_loa_digidconfig_loa_claim_and_more", + ), + ] + + operations = [ + migrations.AddField( + model_name="digidconfig", + name="loa_value_mapping", + field=django_jsonform.models.fields.JSONField( + default=list, + blank=True, + help_text="Level of assurance claim value mappings. Useful if the values in the LOA claim are proprietary, so you can translate them into their standardized identifiers.", + verbose_name="loa mapping", + ), + ), + migrations.AddField( + model_name="digidmachtigenconfig", + name="loa_value_mapping", + field=django_jsonform.models.fields.JSONField( + default=list, + blank=True, + help_text="Level of assurance claim value mappings. Useful if the values in the LOA claim are proprietary, so you can translate them into their standardized identifiers.", + verbose_name="loa mapping", + ), + ), + migrations.AddField( + model_name="eherkenningbewindvoeringconfig", + name="loa_value_mapping", + field=django_jsonform.models.fields.JSONField( + default=list, + blank=True, + help_text="Level of assurance claim value mappings. Useful if the values in the LOA claim are proprietary, so you can translate them into their standardized identifiers.", + verbose_name="loa mapping", + ), + ), + migrations.AddField( + model_name="eherkenningconfig", + name="loa_value_mapping", + field=django_jsonform.models.fields.JSONField( + default=list, + blank=True, + help_text="Level of assurance claim value mappings. Useful if the values in the LOA claim are proprietary, so you can translate them into their standardized identifiers.", + verbose_name="loa mapping", + ), + ), + ] diff --git a/digid_eherkenning/oidc/models/base.py b/digid_eherkenning/oidc/models/base.py index 0eb2914..2efdd89 100644 --- a/digid_eherkenning/oidc/models/base.py +++ b/digid_eherkenning/oidc/models/base.py @@ -1,9 +1,12 @@ +from copy import deepcopy + from django.conf import settings from django.db import models from django.utils.functional import classproperty from django.utils.module_loading import import_string from django.utils.translation import gettext_lazy as _ +from django_jsonform.models.fields import JSONField from mozilla_django_oidc_db.fields import ClaimField from mozilla_django_oidc_db.models import OpenIDConnectConfigBase @@ -23,14 +26,53 @@ def get_default_scopes_kvk(): def default_loa_choices(choicesCls: type[models.TextChoices]): - def decorator(cls: type[models.Model]): - field = cls._meta.get_field("default_loa") - field.choices = choicesCls.choices + def decorator(cls: type[BaseConfig]): + # set the choices for the default_loa + default_loa_field = cls._meta.get_field("default_loa") + assert isinstance(default_loa_field, models.CharField) + default_loa_field.choices = choicesCls.choices + + # specify the choices for the JSONField schema + loa_mapping_field = cls._meta.get_field("loa_value_mapping") + assert isinstance(loa_mapping_field, JSONField) + new_schema = deepcopy(loa_mapping_field.schema) + new_schema["items"]["properties"]["to"]["choices"] = [ + {"value": val, "title": label} for val, label in choicesCls.choices + ] + loa_mapping_field.schema = new_schema + return cls return decorator +LOA_MAPPING_SCHEMA = { + "type": "array", + "items": { + "type": "object", + "required": ["from", "to"], + "properties": { + "from": { + "anyOf": [ + { + "type": "string", + "title": _("String value"), + }, + { + "type": "number", + "title": _("Number value"), + }, + ], + }, + "to": { + "type": "string", + }, + }, + "additionalProperties": False, + }, +} + + class BaseConfig(OpenIDConnectConfigBase): """ Base configuration for DigiD/eHerkenning authentication via OpenID Connect. @@ -57,6 +99,18 @@ class BaseConfig(OpenIDConnectConfigBase): ), ) + loa_value_mapping = JSONField( + _("loa mapping"), + schema=LOA_MAPPING_SCHEMA, + default=list, + blank=True, + help_text=_( + "Level of assurance claim value mappings. Useful if the values in the LOA " + "claim are proprietary, so you can translate them into their standardized " + "identifiers." + ), + ) + class Meta: abstract = True From 40f5fce7b8189a57c5b4d8ce507b81c93e4a8b98 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 21 Jun 2024 14:44:00 +0200 Subject: [PATCH 04/11] :sparkles: Implement claim processing for DigiDConfig --- digid_eherkenning/oidc/claims.py | 55 +++++++++++++++++++ digid_eherkenning/oidc/models/base.py | 8 +++ digid_eherkenning/oidc/models/digid.py | 8 +++ tests/oidc/test_claim_processing.py | 73 ++++++++++++++++++++++++++ 4 files changed, 144 insertions(+) create mode 100644 digid_eherkenning/oidc/claims.py create mode 100644 tests/oidc/test_claim_processing.py diff --git a/digid_eherkenning/oidc/claims.py b/digid_eherkenning/oidc/claims.py new file mode 100644 index 0000000..8c3e9c1 --- /dev/null +++ b/digid_eherkenning/oidc/claims.py @@ -0,0 +1,55 @@ +from glom import Path, PathAccessError, glom +from mozilla_django_oidc_db.typing import ClaimPath, JSONObject + +from .models import BaseConfig + + +def process_claims(claims: JSONObject, config: BaseConfig) -> JSONObject: + """ + Given the raw claims, process them using the provided config. + + Claim processing performs the following steps: + + * Claim name normalization, the provided config model field names are used as keys + * Extracting required and optional values. An error is thrown for missing required + claims, unless a default value is specified in the config. + * Claim value post-processing - if values need to be translated/normalized, the + provided configuration is used. + + The return value SHOULD include the `loa_claim` key, but if no value is available ( + not in the claims and no default specified -> then it's omitted), the key will be + absent. + """ + processed_claims = {} + + # first, extract all the configured required claims + for claim_config in config.CLAIMS_CONFIGURATION: + field_name = claim_config["field"] + path_bits: ClaimPath = getattr(config, field_name) + try: + value = glom(claims, Path(*path_bits)) + except PathAccessError as exc: + if not claim_config["required"]: + continue + claim_repr = " > ".join(path_bits) + raise ValueError(f"Required claim '{claim_repr}' not found") from exc + + processed_claims[field_name] = value + + # then, loa is hardcoded in the base model, process those... + try: + loa = glom(claims, Path(*config.loa_claim)) + loa_claim_missing = False + except PathAccessError: + # default could be empty (string)! + loa = config.default_loa + loa_claim_missing = not loa + + # 'from' is string or number, which are valid keys + loa_map = {mapping["from"]: mapping["to"] for mapping in config.loa_value_mapping} + + if not loa_claim_missing: + # apply mapping, if not found -> use the literal original value instead + processed_claims["loa_claim"] = loa_map.get(loa, loa) + + return processed_claims diff --git a/digid_eherkenning/oidc/models/base.py b/digid_eherkenning/oidc/models/base.py index 2efdd89..66c98c9 100644 --- a/digid_eherkenning/oidc/models/base.py +++ b/digid_eherkenning/oidc/models/base.py @@ -1,4 +1,5 @@ from copy import deepcopy +from typing import TypedDict from django.conf import settings from django.db import models @@ -73,6 +74,11 @@ def decorator(cls: type[BaseConfig]): } +class ClaimConfiguration(TypedDict): + field: str # model field name + required: bool + + class BaseConfig(OpenIDConnectConfigBase): """ Base configuration for DigiD/eHerkenning authentication via OpenID Connect. @@ -111,6 +117,8 @@ class BaseConfig(OpenIDConnectConfigBase): ), ) + CLAIMS_CONFIGURATION: tuple[ClaimConfiguration, ...] + class Meta: abstract = True diff --git a/digid_eherkenning/oidc/models/digid.py b/digid_eherkenning/oidc/models/digid.py index 5f6f99b..753127e 100644 --- a/digid_eherkenning/oidc/models/digid.py +++ b/digid_eherkenning/oidc/models/digid.py @@ -33,6 +33,8 @@ class DigiDConfig(BaseConfig): ), ) + CLAIMS_CONFIGURATION = ({"field": "bsn_claim", "required": True},) + class Meta: verbose_name = _("OpenID Connect configuration for DigiD") @@ -74,6 +76,12 @@ class DigiDMachtigenConfig(BaseConfig): ), ) + CLAIMS_CONFIGURATION = ( + {"field": "representee_bsn_claim", "required": True}, + {"field": "authorizee_bsn_claim", "required": True}, + {"field": "mandate_service_id_claim", "required": True}, + ) + class Meta: verbose_name = _("OpenID Connect configuration for DigiD Machtigen") diff --git a/tests/oidc/test_claim_processing.py b/tests/oidc/test_claim_processing.py new file mode 100644 index 0000000..946e564 --- /dev/null +++ b/tests/oidc/test_claim_processing.py @@ -0,0 +1,73 @@ +import pytest +from mozilla_django_oidc_db.typing import JSONObject + +from digid_eherkenning.choices import DigiDAssuranceLevels +from digid_eherkenning.oidc.claims import process_claims +from digid_eherkenning.oidc.models import ( + DigiDConfig, + DigiDMachtigenConfig, + EHerkenningBewindvoeringConfig, + EHerkenningConfig, +) + +### PLAIN DIGID + + +@pytest.mark.parametrize( + "claims,expected", + [ + # BSN extraction + transform loa values + ( + {"sub": "XXXXXXX54", "authsp_level": "30", "extra": "irrelevant"}, + { + "bsn_claim": "XXXXXXX54", + "loa_claim": DigiDAssuranceLevels.high, + }, + ), + # BSN extraction + missing loa claim + ( + {"sub": "XXXXXXX54"}, + { + "bsn_claim": "XXXXXXX54", + "loa_claim": DigiDAssuranceLevels.middle, + }, + ), + # BSN extraction + unmapped LOA value + ( + {"sub": "XXXXXXX54", "authsp_level": "20", "extra": "irrelevant"}, + { + "bsn_claim": "XXXXXXX54", + "loa_claim": "20", + }, + ), + ], +) +def test_digid_claim_processing(claims: JSONObject, expected: JSONObject): + config = DigiDConfig( + bsn_claim=["sub"], + loa_claim=["authsp_level"], + default_loa=DigiDAssuranceLevels.middle, + loa_value_mapping=[ + {"from": "30", "to": DigiDAssuranceLevels.high}, + ], + ) + + result = process_claims(claims, config) + + assert result == expected + + +def test_digid_raises_on_missing_claims(): + config = DigiDConfig(bsn_claim=["sub"], loa_claim=["authsp_level"]) + + with pytest.raises(ValueError): + process_claims({"bsn": "XXXXXXX54"}, config) + + +def test_loa_claim_absent_without_default_loa(): + config = DigiDConfig(bsn_claim=["sub"], loa_claim=["loa"], default_loa="") + claims: JSONObject = {"sub": "XXXXXXX54"} + + result = process_claims(claims, config) + + assert result == {"bsn_claim": "XXXXXXX54"} From c24df7db981084910d9bb9e34e6236ff57a44b8e Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 21 Jun 2024 15:12:52 +0200 Subject: [PATCH 05/11] :white_check_mark: Add tests for DigiD Machtigen claim processing --- tests/oidc/test_claim_processing.py | 104 ++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/tests/oidc/test_claim_processing.py b/tests/oidc/test_claim_processing.py index 946e564..0b773f8 100644 --- a/tests/oidc/test_claim_processing.py +++ b/tests/oidc/test_claim_processing.py @@ -71,3 +71,107 @@ def test_loa_claim_absent_without_default_loa(): result = process_claims(claims, config) assert result == {"bsn_claim": "XXXXXXX54"} + + +### DIGID MACHTIGEN + + +@pytest.mark.parametrize( + "claims,expected", + [ + # BSN extraction + transform loa values + ( + { + "representee": "XXXXXXX54", + "authorizee": "XXXXXXX99", + "authsp_level": "30", + "service_id": "46ddda34-c4db-4a54-997c-351bc9a0aabc", + "extra": "irrelevant", + }, + { + "representee_bsn_claim": "XXXXXXX54", + "authorizee_bsn_claim": "XXXXXXX99", + "loa_claim": DigiDAssuranceLevels.high, + "mandate_service_id_claim": "46ddda34-c4db-4a54-997c-351bc9a0aabc", + }, + ), + # BSN extraction + missing loa claim + ( + { + "representee": "XXXXXXX54", + "authorizee": "XXXXXXX99", + "service_id": "46ddda34-c4db-4a54-997c-351bc9a0aabc", + }, + { + "representee_bsn_claim": "XXXXXXX54", + "authorizee_bsn_claim": "XXXXXXX99", + "loa_claim": DigiDAssuranceLevels.middle, + "mandate_service_id_claim": "46ddda34-c4db-4a54-997c-351bc9a0aabc", + }, + ), + # BSN extraction + unmapped LOA value + ( + { + "representee": "XXXXXXX54", + "authorizee": "XXXXXXX99", + "authsp_level": "20", + "service_id": "46ddda34-c4db-4a54-997c-351bc9a0aabc", + "extra": "irrelevant", + }, + { + "representee_bsn_claim": "XXXXXXX54", + "authorizee_bsn_claim": "XXXXXXX99", + "loa_claim": "20", + "mandate_service_id_claim": "46ddda34-c4db-4a54-997c-351bc9a0aabc", + }, + ), + ], +) +def test_digid_machtigen_claim_processing(claims: JSONObject, expected: JSONObject): + config = DigiDMachtigenConfig( + representee_bsn_claim=["representee"], + authorizee_bsn_claim=["authorizee"], + mandate_service_id_claim=["service_id"], + loa_claim=["authsp_level"], + default_loa=DigiDAssuranceLevels.middle, + loa_value_mapping=[ + {"from": "30", "to": DigiDAssuranceLevels.high}, + ], + ) + + result = process_claims(claims, config) + + assert result == expected + + +@pytest.mark.parametrize( + "claims", + ( + {}, + { + "authorizee": "XXXXXXX99", + "authsp_level": "30", + "service_id": "46ddda34-c4db-4a54-997c-351bc9a0aabc", + }, + { + "representee": "XXXXXXX54", + "authsp_level": "30", + "service_id": "46ddda34-c4db-4a54-997c-351bc9a0aabc", + }, + { + "representee": "XXXXXXX54", + "authorizee": "XXXXXXX99", + "authsp_level": "30", + }, + ), +) +def test_digid_machtigen_raises_on_missing_claims(claims: JSONObject): + config = DigiDMachtigenConfig( + representee_bsn_claim=["representee"], + authorizee_bsn_claim=["authorizee"], + mandate_service_id_claim=["service_id"], + loa_claim=["authsp_level"], + ) + + with pytest.raises(ValueError): + process_claims(claims, config) From dc2fe048adcf86ba146ef993b26763a3c05aeadf Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 21 Jun 2024 15:49:51 +0200 Subject: [PATCH 06/11] :white_check_mark: Add tests for eherkenning claim processing --- digid_eherkenning/oidc/models/eherkenning.py | 15 +++ tests/oidc/test_claim_processing.py | 103 ++++++++++++++++++- 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/digid_eherkenning/oidc/models/eherkenning.py b/digid_eherkenning/oidc/models/eherkenning.py index 142619b..62a76e5 100644 --- a/digid_eherkenning/oidc/models/eherkenning.py +++ b/digid_eherkenning/oidc/models/eherkenning.py @@ -17,6 +17,8 @@ class AuthorizeeMixin(models.Model): + # XXX: this may require a value mapping, depending on what brokers return + # XXX: this may require a fallback value, depending on what brokers return identifier_type_claim = ClaimField( verbose_name=_("identifier type claim"), # XXX: Anoigo specific default @@ -53,6 +55,13 @@ class AuthorizeeMixin(models.Model): ), ) + CLAIMS_CONFIGURATION = ( + {"field": "identifier_type_claim", "required": False}, + {"field": "legal_subject_claim", "required": True}, + {"field": "acting_subject_claim", "required": True}, + {"field": "branch_number_claim", "required": False}, + ) + class Meta: abstract = True @@ -128,6 +137,12 @@ class EHerkenningBewindvoeringConfig(AuthorizeeMixin, BaseConfig): ), ) + CLAIMS_CONFIGURATION = AuthorizeeMixin.CLAIMS_CONFIGURATION + ( + {"field": "representee_claim", "required": True}, + {"field": "mandate_service_id_claim", "required": True}, + {"field": "mandate_service_uuid_claim", "required": True}, + ) + class Meta: verbose_name = _("OpenID Connect configuration for eHerkenning Bewindvoering") diff --git a/tests/oidc/test_claim_processing.py b/tests/oidc/test_claim_processing.py index 0b773f8..e5c1fb0 100644 --- a/tests/oidc/test_claim_processing.py +++ b/tests/oidc/test_claim_processing.py @@ -1,7 +1,7 @@ import pytest from mozilla_django_oidc_db.typing import JSONObject -from digid_eherkenning.choices import DigiDAssuranceLevels +from digid_eherkenning.choices import AssuranceLevels, DigiDAssuranceLevels from digid_eherkenning.oidc.claims import process_claims from digid_eherkenning.oidc.models import ( DigiDConfig, @@ -175,3 +175,104 @@ def test_digid_machtigen_raises_on_missing_claims(claims: JSONObject): with pytest.raises(ValueError): process_claims(claims, config) + + +### EHERKENNING + + +@pytest.mark.parametrize( + "claims,expected", + [ + # all claims provided, happy flow + ( + { + "namequalifier": "urn:etoegang:1.9:EntityConcernedID:KvKnr", + "kvk": "12345678", + "sub": "-opaquestring-", + "vestiging": "123456789012", + "loa": "urn:etoegang:core:assurance-class:loa2plus", + "extra": "ignored", + }, + { + "identifier_type_claim": "urn:etoegang:1.9:EntityConcernedID:KvKnr", + "legal_subject_claim": "12345678", + "acting_subject_claim": "-opaquestring-", + "branch_number_claim": "123456789012", + "loa_claim": "urn:etoegang:core:assurance-class:loa2plus", + }, + ), + # all required claims provided, happy flow + ( + { + "kvk": "12345678", + "sub": "-opaquestring-", + "loa": "urn:etoegang:core:assurance-class:loa2plus", + }, + { + "legal_subject_claim": "12345678", + "acting_subject_claim": "-opaquestring-", + "loa_claim": "urn:etoegang:core:assurance-class:loa2plus", + }, + ), + # mapping loa value + ( + { + "kvk": "12345678", + "sub": "-opaquestring-", + "loa": 3, + }, + { + "legal_subject_claim": "12345678", + "acting_subject_claim": "-opaquestring-", + "loa_claim": "urn:etoegang:core:assurance-class:loa3", + }, + ), + # default/fallback loa + ( + { + "kvk": "12345678", + "sub": "-opaquestring-", + }, + { + "legal_subject_claim": "12345678", + "acting_subject_claim": "-opaquestring-", + "loa_claim": "urn:etoegang:core:assurance-class:loa2plus", + }, + ), + ], +) +def test_eherkenning_claim_processing(claims: JSONObject, expected: JSONObject): + config = EHerkenningConfig( + identifier_type_claim=["namequalifier"], + legal_subject_claim=["kvk"], + acting_subject_claim=["sub"], + branch_number_claim=["vestiging"], + loa_claim=["loa"], + default_loa=AssuranceLevels.low_plus, + loa_value_mapping=[ + {"from": 3, "to": AssuranceLevels.substantial}, + ], + ) + + result = process_claims(claims, config) + + assert result == expected + + +@pytest.mark.parametrize( + "claims", + [ + {"kvk": "12345678"}, + {"sub": "-opaquestring-"}, + ], +) +def test_eherkenning_raises_on_missing_claims(claims: JSONObject): + config = EHerkenningConfig( + identifier_type_claim=["namequalifier"], + legal_subject_claim=["kvk"], + acting_subject_claim=["sub"], + branch_number_claim=["vestiging"], + ) + + with pytest.raises(ValueError): + process_claims(claims, config) From 0a32e3136d72e9f571116c915485344ac9750bca Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 21 Jun 2024 15:56:39 +0200 Subject: [PATCH 07/11] :white_check_mark: Tests for eh-bewindvoering claim processing --- tests/oidc/test_claim_processing.py | 161 ++++++++++++++++++++++++++++ 1 file changed, 161 insertions(+) diff --git a/tests/oidc/test_claim_processing.py b/tests/oidc/test_claim_processing.py index e5c1fb0..ea59981 100644 --- a/tests/oidc/test_claim_processing.py +++ b/tests/oidc/test_claim_processing.py @@ -276,3 +276,164 @@ def test_eherkenning_raises_on_missing_claims(claims: JSONObject): with pytest.raises(ValueError): process_claims(claims, config) + + +# EHERKENNING BEWINDVOERING + + +@pytest.mark.parametrize( + "claims,expected", + [ + # all claims provided, happy flow + ( + { + "namequalifier": "urn:etoegang:1.9:EntityConcernedID:KvKnr", + "kvk": "12345678", + "sub": "-opaquestring-", + "vestiging": "123456789012", + "loa": "urn:etoegang:core:assurance-class:loa2plus", + "bsn": "XXXXXXX54", + "service_id": "urn:etoegang:DV:00000001002308836000:services:9113", + "service_uuid": "34085d78-21aa-4481-a219-b28d7f3282fc", + "extra": "ignored", + }, + { + "identifier_type_claim": "urn:etoegang:1.9:EntityConcernedID:KvKnr", + "legal_subject_claim": "12345678", + "acting_subject_claim": "-opaquestring-", + "branch_number_claim": "123456789012", + "loa_claim": "urn:etoegang:core:assurance-class:loa2plus", + "representee_claim": "XXXXXXX54", + "mandate_service_id_claim": "urn:etoegang:DV:00000001002308836000:services:9113", + "mandate_service_uuid_claim": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + ), + # all required claims provided, happy flow + ( + { + "kvk": "12345678", + "sub": "-opaquestring-", + "loa": "urn:etoegang:core:assurance-class:loa2plus", + "bsn": "XXXXXXX54", + "service_id": "urn:etoegang:DV:00000001002308836000:services:9113", + "service_uuid": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + { + "legal_subject_claim": "12345678", + "acting_subject_claim": "-opaquestring-", + "loa_claim": "urn:etoegang:core:assurance-class:loa2plus", + "representee_claim": "XXXXXXX54", + "mandate_service_id_claim": "urn:etoegang:DV:00000001002308836000:services:9113", + "mandate_service_uuid_claim": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + ), + # mapping loa value + ( + { + "kvk": "12345678", + "sub": "-opaquestring-", + "loa": 3, + "bsn": "XXXXXXX54", + "service_id": "urn:etoegang:DV:00000001002308836000:services:9113", + "service_uuid": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + { + "legal_subject_claim": "12345678", + "acting_subject_claim": "-opaquestring-", + "loa_claim": "urn:etoegang:core:assurance-class:loa3", + "representee_claim": "XXXXXXX54", + "mandate_service_id_claim": "urn:etoegang:DV:00000001002308836000:services:9113", + "mandate_service_uuid_claim": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + ), + # default/fallback loa + ( + { + "kvk": "12345678", + "sub": "-opaquestring-", + "bsn": "XXXXXXX54", + "service_id": "urn:etoegang:DV:00000001002308836000:services:9113", + "service_uuid": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + { + "legal_subject_claim": "12345678", + "acting_subject_claim": "-opaquestring-", + "loa_claim": "urn:etoegang:core:assurance-class:loa2plus", + "representee_claim": "XXXXXXX54", + "mandate_service_id_claim": "urn:etoegang:DV:00000001002308836000:services:9113", + "mandate_service_uuid_claim": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + ), + ], +) +def test_eherkenning_bewindvoering_claim_processing( + claims: JSONObject, expected: JSONObject +): + config = EHerkenningBewindvoeringConfig( + identifier_type_claim=["namequalifier"], + legal_subject_claim=["kvk"], + acting_subject_claim=["sub"], + branch_number_claim=["vestiging"], + representee_claim=["bsn"], + mandate_service_id_claim=["service_id"], + mandate_service_uuid_claim=["service_uuid"], + loa_claim=["loa"], + default_loa=AssuranceLevels.low_plus, + loa_value_mapping=[ + {"from": 3, "to": AssuranceLevels.substantial}, + ], + ) + + result = process_claims(claims, config) + + assert result == expected + + +@pytest.mark.parametrize( + "claims", + [ + { + "kvk": "12345678", + "bsn": "XXXXXXX54", + "service_id": "urn:etoegang:DV:00000001002308836000:services:9113", + "service_uuid": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + { + "sub": "-opaquestring-", + "bsn": "XXXXXXX54", + "service_id": "urn:etoegang:DV:00000001002308836000:services:9113", + "service_uuid": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + { + "kvk": "12345678", + "sub": "-opaquestring-", + "service_id": "urn:etoegang:DV:00000001002308836000:services:9113", + "service_uuid": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + { + "kvk": "12345678", + "sub": "-opaquestring-", + "bsn": "XXXXXXX54", + "service_uuid": "34085d78-21aa-4481-a219-b28d7f3282fc", + }, + { + "kvk": "12345678", + "sub": "-opaquestring-", + "bsn": "XXXXXXX54", + "service_id": "urn:etoegang:DV:00000001002308836000:services:9113", + }, + ], +) +def test_eherkenning_bewindvoering_raises_on_missing_claims(claims: JSONObject): + config = EHerkenningBewindvoeringConfig( + identifier_type_claim=["namequalifier"], + legal_subject_claim=["kvk"], + acting_subject_claim=["sub"], + branch_number_claim=["vestiging"], + representee_claim=["bsn"], + mandate_service_id_claim=["service_id"], + mandate_service_uuid_claim=["service_uuid"], + ) + + with pytest.raises(ValueError): + process_claims(claims, config) From 0e6e29921f10be77a9940af14397b66e9396c92f Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 21 Jun 2024 16:03:46 +0200 Subject: [PATCH 08/11] :pencil: Document OIDC claim processing helper --- digid_eherkenning/oidc/claims.py | 4 ++-- docs/conf.py | 13 ++++++++++--- docs/oidc.rst | 5 +++++ setup.cfg | 1 + 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/digid_eherkenning/oidc/claims.py b/digid_eherkenning/oidc/claims.py index 8c3e9c1..5beeb29 100644 --- a/digid_eherkenning/oidc/claims.py +++ b/digid_eherkenning/oidc/claims.py @@ -16,8 +16,8 @@ def process_claims(claims: JSONObject, config: BaseConfig) -> JSONObject: * Claim value post-processing - if values need to be translated/normalized, the provided configuration is used. - The return value SHOULD include the `loa_claim` key, but if no value is available ( - not in the claims and no default specified -> then it's omitted), the key will be + The return value SHOULD include the ``loa_claim`` key, but if no value is available + (not in the claims and no default specified -> then it's omitted), the key will be absent. """ processed_claims = {} diff --git a/docs/conf.py b/docs/conf.py index 8ded3f6..0df50c0 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -6,6 +6,8 @@ # -- Path setup -------------------------------------------------------------- +import os + # If extensions (or modules to document with autodoc) are in another directory, # add these directories to sys.path here. If the directory is relative to the # documentation root, use os.path.abspath to make it absolute, like shown here. @@ -13,10 +15,15 @@ import sys from pathlib import Path -current_dir = Path(__file__).parent.parent -code_directory = current_dir / "digid_eherkenning" +import django + +repo_root = Path(__file__).parent.parent.resolve() +sys.path.insert(0, str(repo_root)) + +os.environ.setdefault("DJANGO_SETTINGS_MODULE", "tests.project.settings") +os.environ.setdefault("OIDC_ENABLED", "yes") -sys.path.insert(0, str(code_directory)) +django.setup() # -- Project information ----------------------------------------------------- diff --git a/docs/oidc.rst b/docs/oidc.rst index f4c6d01..2071032 100644 --- a/docs/oidc.rst +++ b/docs/oidc.rst @@ -71,3 +71,8 @@ incorporate in your own initialization flow views: * :attr:`digid_eherkenning.oidc.views.eh_bewindvoering_init` .. _mozilla-django-oidc-db: https://mozilla-django-oidc-db.readthedocs.io/en/latest/ + +Claim processing +================ + +.. autofunction:: digid_eherkenning.oidc.claims.process_claims diff --git a/setup.cfg b/setup.cfg index b44729d..caa70f5 100644 --- a/setup.cfg +++ b/setup.cfg @@ -76,6 +76,7 @@ coverage = pytest-cov docs = sphinx sphinx-rtd-theme + psycopg2 release = bump2version twine From 4babb670ac27d7839ad3ea9a7013e81b549b1eac Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 21 Jun 2024 16:07:32 +0200 Subject: [PATCH 09/11] :pencil: Document new features in changelog --- CHANGELOG.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b810091..d33389e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,6 +8,12 @@ Changelog Further iteration on the OIDC integration. * 💥⚠️ Renamed the ``OpenIDConnectBaseConfig`` base model to ``BaseConfig`` +* Added "level of assurance" claim configuration +* Added ability to specify a fallback LOA value +* Added ability to map claim values to their standard values +* Added ``digid_eherkenning.oidc.claims.process_claims`` helper to normalize received + claims from the OIDC provider for further processing. See the tests for the intended + behaviour. 0.14.0 (2024-06-13) =================== From 2495107f8f3516eb416d26ba1c5dea9e7e96de08 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Fri, 21 Jun 2024 17:36:30 +0200 Subject: [PATCH 10/11] :sparkles: Handle missing loa_claim configuration --- digid_eherkenning/oidc/claims.py | 46 ++++++++++++++++++++++++----- tests/oidc/test_claim_processing.py | 20 ++++++++++++- 2 files changed, 57 insertions(+), 9 deletions(-) diff --git a/digid_eherkenning/oidc/claims.py b/digid_eherkenning/oidc/claims.py index 5beeb29..3aaeb70 100644 --- a/digid_eherkenning/oidc/claims.py +++ b/digid_eherkenning/oidc/claims.py @@ -1,8 +1,16 @@ +import logging + from glom import Path, PathAccessError, glom from mozilla_django_oidc_db.typing import ClaimPath, JSONObject from .models import BaseConfig +logger = logging.getLogger(__name__) + + +class NoLOAClaim(Exception): + pass + def process_claims(claims: JSONObject, config: BaseConfig) -> JSONObject: """ @@ -37,19 +45,41 @@ def process_claims(claims: JSONObject, config: BaseConfig) -> JSONObject: processed_claims[field_name] = value # then, loa is hardcoded in the base model, process those... + try: + loa = _process_loa(claims, config) + except NoLOAClaim as exc: + logger.info( + "Missing LoA claim, excluding it from processed claims", exc_info=exc + ) + else: + processed_claims["loa_claim"] = loa + + return processed_claims + + +def _process_loa(claims: JSONObject, config: BaseConfig) -> str: + default = config.default_loa + if not (loa_claim := config.loa_claim) and not default: + raise NoLOAClaim("No LoA claim or default LoA configured") + + if not loa_claim: + return default + try: loa = glom(claims, Path(*config.loa_claim)) loa_claim_missing = False except PathAccessError: # default could be empty (string)! - loa = config.default_loa - loa_claim_missing = not loa + loa = default + loa_claim_missing = not default - # 'from' is string or number, which are valid keys - loa_map = {mapping["from"]: mapping["to"] for mapping in config.loa_value_mapping} + if loa_claim_missing: + raise NoLOAClaim("LoA claim is absent and no default LoA configured") - if not loa_claim_missing: - # apply mapping, if not found -> use the literal original value instead - processed_claims["loa_claim"] = loa_map.get(loa, loa) + # 'from' is string or number, which are valid keys + loa_map: dict[str | float | int, str] = { + mapping["from"]: mapping["to"] for mapping in config.loa_value_mapping + } - return processed_claims + # apply mapping, if not found -> use the literal original value instead + return loa_map.get(loa, loa) diff --git a/tests/oidc/test_claim_processing.py b/tests/oidc/test_claim_processing.py index ea59981..6389fa6 100644 --- a/tests/oidc/test_claim_processing.py +++ b/tests/oidc/test_claim_processing.py @@ -64,7 +64,7 @@ def test_digid_raises_on_missing_claims(): process_claims({"bsn": "XXXXXXX54"}, config) -def test_loa_claim_absent_without_default_loa(): +def test_digid_loa_claim_absent_without_default_loa(): config = DigiDConfig(bsn_claim=["sub"], loa_claim=["loa"], default_loa="") claims: JSONObject = {"sub": "XXXXXXX54"} @@ -73,6 +73,24 @@ def test_loa_claim_absent_without_default_loa(): assert result == {"bsn_claim": "XXXXXXX54"} +def test_digid_loa_claim_not_configured_but_default_set(): + config = DigiDConfig(default_loa="middle") + claims: JSONObject = {"bsn": "XXXXXXX54", "loa": "ignored"} + + result = process_claims(claims, config) + + assert result == {"bsn_claim": "XXXXXXX54", "loa_claim": "middle"} + + +def test_digid_claim_processing_with_defaults(): + config = DigiDConfig() + claims: JSONObject = {"bsn": "XXXXXXX54"} + + result = process_claims(claims, config) + + assert result == {"bsn_claim": "XXXXXXX54"} + + ### DIGID MACHTIGEN From 0b17cfe5599412e63404021773992cac9b17eee4 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Mon, 24 Jun 2024 09:54:57 +0200 Subject: [PATCH 11/11] :sparkles: Add strict/lax mode for claim processing In non-strict (lax) mode, absent required claims don't lead to ValueError being raised so that the caller can handle this data appropriately. Required for some backwards-compatibility in Open Forms. --- digid_eherkenning/oidc/claims.py | 21 ++++++++++++++++++++- tests/oidc/test_claim_processing.py | 8 ++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/digid_eherkenning/oidc/claims.py b/digid_eherkenning/oidc/claims.py index 3aaeb70..65f8e27 100644 --- a/digid_eherkenning/oidc/claims.py +++ b/digid_eherkenning/oidc/claims.py @@ -12,7 +12,11 @@ class NoLOAClaim(Exception): pass -def process_claims(claims: JSONObject, config: BaseConfig) -> JSONObject: +def process_claims( + claims: JSONObject, + config: BaseConfig, + strict: bool = True, +) -> JSONObject: """ Given the raw claims, process them using the provided config. @@ -27,6 +31,17 @@ def process_claims(claims: JSONObject, config: BaseConfig) -> JSONObject: The return value SHOULD include the ``loa_claim`` key, but if no value is available (not in the claims and no default specified -> then it's omitted), the key will be absent. + + :arg claims: The raw claims as received from the Identity Provider. + :arg config: The OIDC Configuration instance that specifies which claims should be + extracted and processed. + :arg strict: In strict mode, absent claims that are required (according) to the + configuration raise an error. In non-strict mode, these claims are simply skipped + and omitted. + :returns: A (JSON-serializable) dictionary where the keys are the claim config + field names, taken from ``config.CLAIMS_CONFIGURATION``, and the values their + extracted values from the raw claims. Extracted values have been post-processed + if post-processing configuration was available. """ processed_claims = {} @@ -39,6 +54,10 @@ def process_claims(claims: JSONObject, config: BaseConfig) -> JSONObject: except PathAccessError as exc: if not claim_config["required"]: continue + # in non-strict mode, do not raise but instead omit the claim. Up to the + # caller to handle missing claims. + if not strict: + continue claim_repr = " > ".join(path_bits) raise ValueError(f"Required claim '{claim_repr}' not found") from exc diff --git a/tests/oidc/test_claim_processing.py b/tests/oidc/test_claim_processing.py index 6389fa6..10e134b 100644 --- a/tests/oidc/test_claim_processing.py +++ b/tests/oidc/test_claim_processing.py @@ -91,6 +91,14 @@ def test_digid_claim_processing_with_defaults(): assert result == {"bsn_claim": "XXXXXXX54"} +def test_lax_mode(): + config = DigiDConfig(bsn_claim=["sub"], loa_claim=["authsp_level"]) + + result = process_claims({"bsn": "XXXXXXX54"}, config, strict=False) + + assert result == {} + + ### DIGID MACHTIGEN