From 543157b547fc0cde8cbb44f98c91745b001a5ec3 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 12 Jun 2024 11:32:13 +0200 Subject: [PATCH 1/7] :arrow_up: Bump minimum required moz-oidc-db library version Version 0.18.0 will have the ClaimFieldDefault utility that we rely on. --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index f23e6c3..c69b8f9 100644 --- a/setup.cfg +++ b/setup.cfg @@ -60,7 +60,7 @@ include = [options.extras_require] oidc = - mozilla-django-oidc-db >= 0.17.0 + mozilla-django-oidc-db >= 0.18.0 tests = django-test-migrations pytest From ec8615016de996a514e2d5cfebed4396f60032e3 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 12 Jun 2024 11:47:18 +0200 Subject: [PATCH 2/7] :card_file_box: Migrate claim fields to proper ClaimField and add missing fields The existing string-fields are now migrated to claim fields so that dots in claim names are supported. Additionally, the fields have been renamed to better cover their meaning, in line with the authentication context data model/json schema. --- .../0004_migrate_config_to_claimfield.py | 258 ++++++++++++++++++ digid_eherkenning/oidc/models/digid.py | 58 ++-- digid_eherkenning/oidc/models/eherkenning.py | 113 +++++--- 3 files changed, 371 insertions(+), 58 deletions(-) create mode 100644 digid_eherkenning/oidc/migrations/0004_migrate_config_to_claimfield.py diff --git a/digid_eherkenning/oidc/migrations/0004_migrate_config_to_claimfield.py b/digid_eherkenning/oidc/migrations/0004_migrate_config_to_claimfield.py new file mode 100644 index 0000000..b4748d0 --- /dev/null +++ b/digid_eherkenning/oidc/migrations/0004_migrate_config_to_claimfield.py @@ -0,0 +1,258 @@ +# Generated by Django 4.2.13 on 2024-06-11 13:43 + +from typing import Any, Callable + +from django.conf import settings +from django.core.cache import caches +from django.db import migrations, models, transaction + +import mozilla_django_oidc_db.fields + + +def flush_cache(): + cache_name = getattr(settings, "SOLO_CACHE", None) + if not cache_name: + return + caches[cache_name].clear() + + +def operation_factory(model: str, mappings: dict[str, str]) -> migrations.RunPython: + + def _action_factory(transformer: Callable[[Any], None]): + def _run_python_action(apps, _) -> None: + ConfigModel = apps.get_model("digid_eherkenning_oidc_generics", model) + + # Solo model, so there's only ever one instance + config = ConfigModel.objects.first() + if config is None: + return + + transformer(config) + + config.save() + transaction.on_commit(flush_cache) + + return _run_python_action + + @_action_factory + def forward(instance) -> None: + for old, new in mappings.items(): + new_value = getattr(instance, old).split(".") + setattr(instance, new, new_value) + + @_action_factory + def reverse(instance) -> None: + for old, new in mappings.items(): + old_value = ".".join(getattr(instance, new)) + setattr(instance, old, old_value) + + return migrations.RunPython(forward, reverse) + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "digid_eherkenning_oidc_generics", + "0003_rename_openidconnectpublicconfig_digidconfig_and_more", + ), + ] + + operations = [ + migrations.AddField( + model_name="digidconfig", + name="bsn_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault("bsn"), + help_text="Name of the claim holding the authenticated user's BSN.", + size=None, + verbose_name="bsn claim", + ), + ), + migrations.AddField( + model_name="digidmachtigenconfig", + name="authorizee_bsn_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:nl-eid-gdi:1.0:ActingSubjectID" + ), + help_text="Name of the claim holding the BSN of the authorized user.", + size=None, + verbose_name="authorizee bsn claim", + ), + ), + migrations.AddField( + model_name="digidmachtigenconfig", + name="representee_bsn_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:nl-eid-gdi:1.0:LegalSubjectID" + ), + help_text="Name of the claim holding the BSN of the represented user.", + size=None, + verbose_name="representee bsn claim", + ), + ), + migrations.AddField( + model_name="eherkenningbewindvoeringconfig", + name="identifier_type_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "namequalifier" + ), + help_text="Claim that specifies how the legal subject claim must be interpreted. The expected claim value is one of: 'urn:etoegang:1.9:EntityConcernedID:KvKnr' or 'urn:etoegang:1.9:EntityConcernedID:RSIN'.", + size=None, + verbose_name="identifier type claim", + ), + ), + migrations.AddField( + model_name="eherkenningbewindvoeringconfig", + name="legal_subject_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:etoegang:core:LegalSubjectID" + ), + help_text="Name of the claim holding the identifier of the authenticated company.", + size=None, + verbose_name="company identifier claim", + ), + ), + migrations.AddField( + model_name="eherkenningbewindvoeringconfig", + name="representee_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault("sel_uid"), + help_text="Name of the claim holding the identifier (like a BSN, RSIN or CoC number) of the represented person/company.", + size=None, + verbose_name="representee identifier claim", + ), + ), + migrations.AddField( + model_name="eherkenningconfig", + name="identifier_type_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "namequalifier" + ), + help_text="Claim that specifies how the legal subject claim must be interpreted. The expected claim value is one of: 'urn:etoegang:1.9:EntityConcernedID:KvKnr' or 'urn:etoegang:1.9:EntityConcernedID:RSIN'.", + size=None, + verbose_name="identifier type claim", + ), + ), + migrations.AddField( + model_name="eherkenningconfig", + name="legal_subject_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:etoegang:core:LegalSubjectID" + ), + help_text="Name of the claim holding the identifier of the authenticated company.", + size=None, + verbose_name="company identifier claim", + ), + ), + migrations.AddField( + model_name="eherkenningbewindvoeringconfig", + name="acting_subject_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:etoegang:core:ActingSubjectID" + ), + help_text="Name of the claim holding the (opaque) identifier of the user representing the authenticated company..", + size=None, + verbose_name="acting subject identifier claim", + ), + ), + migrations.AddField( + model_name="eherkenningconfig", + name="acting_subject_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:etoegang:core:ActingSubjectID" + ), + help_text="Name of the claim holding the (opaque) identifier of the user representing the authenticated company..", + size=None, + verbose_name="acting subject identifier claim", + ), + ), + operation_factory( + "DigiDConfig", + mappings={ + "identifier_claim_name": "bsn_claim", + }, + ), + operation_factory( + "DigiDMachtigenConfig", + mappings={ + "vertegenwoordigde_claim_name": "representee_bsn_claim", + "gemachtigde_claim_name": "authorizee_bsn_claim", + }, + ), + operation_factory( + "EHerkenningConfig", + mappings={ + "identifier_claim_name": "legal_subject_claim", + }, + ), + operation_factory( + "EHerkenningBewindvoeringConfig", + mappings={ + "vertegenwoordigde_company_claim_name": "representee_claim", + "gemachtigde_person_claim_name": "acting_subject_claim", + }, + ), + migrations.RemoveField( + model_name="digidconfig", + name="identifier_claim_name", + ), + migrations.RemoveField( + model_name="digidmachtigenconfig", + name="gemachtigde_claim_name", + ), + migrations.RemoveField( + model_name="digidmachtigenconfig", + name="vertegenwoordigde_claim_name", + ), + migrations.RemoveField( + model_name="eherkenningbewindvoeringconfig", + name="gemachtigde_person_claim_name", + ), + migrations.RemoveField( + model_name="eherkenningbewindvoeringconfig", + name="vertegenwoordigde_company_claim_name", + ), + migrations.RemoveField( + model_name="eherkenningconfig", + name="identifier_claim_name", + ), + ] diff --git a/digid_eherkenning/oidc/models/digid.py b/digid_eherkenning/oidc/models/digid.py index e76b4e5..aa93c62 100644 --- a/digid_eherkenning/oidc/models/digid.py +++ b/digid_eherkenning/oidc/models/digid.py @@ -4,6 +4,7 @@ from django.utils.translation import gettext_lazy as _ from django_jsonform.models.fields import ArrayField +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 @@ -14,11 +15,10 @@ class DigiDConfig(OpenIDConnectBaseConfig): Configuration for DigiD authentication via OpenID connect """ - identifier_claim_name = models.CharField( - _("BSN claim name"), - max_length=100, - help_text=_("The name of the claim in which the BSN of the user is stored"), - default="bsn", + bsn_claim = ClaimField( + verbose_name=_("bsn claim"), + default=ClaimFieldDefault("bsn"), + help_text=_("Name of the claim holding the authenticated user's BSN."), ) oidc_rp_scopes_list = ArrayField( verbose_name=_("OpenID Connect scopes"), @@ -27,7 +27,7 @@ class DigiDConfig(OpenIDConnectBaseConfig): blank=True, help_text=_( "OpenID Connect scopes that are requested during login. " - "These scopes are hardcoded and must be supported by the identity provider" + "These scopes are hardcoded and must be supported by the identity provider." ), ) @@ -36,27 +36,30 @@ class Meta: @property def oidcdb_username_claim(self) -> ClaimPath: - return [self.identifier_claim_name] + return self.bsn_claim class DigiDMachtigenConfig(OpenIDConnectBaseConfig): - # TODO: support periods in claim keys - vertegenwoordigde_claim_name = models.CharField( - verbose_name=_("vertegenwoordigde claim name"), - default="aanvrager.bsn", - max_length=50, - help_text=_( - "Name of the claim in which the BSN of the person being represented is stored" - ), + # TODO: these default claim names don't appear to be part of any standard... + representee_bsn_claim = ClaimField( + verbose_name=_("representee bsn claim"), + default=ClaimFieldDefault("urn:nl-eid-gdi:1.0:LegalSubjectID"), + help_text=_("Name of the claim holding the BSN of the represented user."), ) - gemachtigde_claim_name = models.CharField( - verbose_name=_("gemachtigde claim name"), - default="gemachtigde.bsn", - max_length=50, - help_text=_( - "Name of the claim in which the BSN of the person representing someone else is stored" - ), + authorizee_bsn_claim = ClaimField( + verbose_name=_("authorizee bsn claim"), + default=ClaimFieldDefault("urn:nl-eid-gdi:1.0:ActingSubjectID"), + help_text=_("Name of the claim holding the BSN of the authorized user."), ) + # mandate_service_id_claim = ClaimField( + # verbose_name=_("service ID claim"), + # default=ClaimFieldDefault("urn:nl-eid-gdi:1.0:ServiceUUID"), + # help_text=_( + # "Name of the claim holding the service UUID for which the acting subject " + # "is authorized." + # ), + # ) + oidc_rp_scopes_list = ArrayField( verbose_name=_("OpenID Connect scopes"), base_field=models.CharField(_("OpenID Connect scope"), max_length=50), @@ -64,7 +67,7 @@ class DigiDMachtigenConfig(OpenIDConnectBaseConfig): blank=True, help_text=_( "OpenID Connect scopes that are requested during login. " - "These scopes are hardcoded and must be supported by the identity provider" + "These scopes are hardcoded and must be supported by the identity provider." ), ) @@ -72,12 +75,13 @@ class Meta: verbose_name = _("OpenID Connect configuration for DigiD Machtigen") @property - def digid_eherkenning_machtigen_claims(self) -> dict[str, ClaimPath]: + def mandate_claims(self) -> dict[str, ClaimPath]: return { - "vertegenwoordigde": [self.vertegenwoordigde_claim_name], - "gemachtigde": [self.gemachtigde_claim_name], + "representee": self.representee_bsn_claim, + "authorizee": self.authorizee_bsn_claim, + "service_id": self.mandate_service_id_claim, } @property def oidcdb_sensitive_claims(self) -> Collection[ClaimPath]: - return list(self.digid_eherkenning_machtigen_claims.values()) + return list(self.mandate_claims.values()) diff --git a/digid_eherkenning/oidc/models/eherkenning.py b/digid_eherkenning/oidc/models/eherkenning.py index 2bec814..dbe9e31 100644 --- a/digid_eherkenning/oidc/models/eherkenning.py +++ b/digid_eherkenning/oidc/models/eherkenning.py @@ -4,6 +4,7 @@ from django.utils.translation import gettext_lazy as _ from django_jsonform.models.fields import ArrayField +from mozilla_django_oidc_db.fields import ClaimField, ClaimFieldDefault from mozilla_django_oidc_db.typing import ClaimPath from .base import ( @@ -13,17 +14,52 @@ ) -class EHerkenningConfig(OpenIDConnectBaseConfig): +class AuthorizeeMixin(models.Model): + identifier_type_claim = ClaimField( + verbose_name=_("identifier type claim"), + # XXX: Anoigo specific default + default=ClaimFieldDefault("namequalifier"), + help_text=_( + "Claim that specifies how the legal subject claim must be interpreted. " + "The expected claim value is one of: " + "'urn:etoegang:1.9:EntityConcernedID:KvKnr' or " + "'urn:etoegang:1.9:EntityConcernedID:RSIN'." + ), + ) + # TODO: what if the claims for kvk/RSIN are different claims names? + legal_subject_claim = ClaimField( + verbose_name=_("company identifier claim"), + default=ClaimFieldDefault("urn:etoegang:core:LegalSubjectID"), + help_text=_( + "Name of the claim holding the identifier of the authenticated company." + ), + ) + acting_subject_claim = ClaimField( + verbose_name=_("acting subject identifier claim"), + default=ClaimFieldDefault("urn:etoegang:core:ActingSubjectID"), + help_text=_( + "Name of the claim holding the (opaque) identifier of the user " + "representing the authenticated company.." + ), + ) + # branch_number_claim = ClaimField( + # verbose_name=_("branch number claim"), + # default=ClaimFieldDefault("urn:etoegang:1.9:ServiceRestriction:Vestigingsnr"), + # help_text=_( + # "Name of the claim holding the value of the branch number for the " + # "authenticated company, if such a restriction applies." + # ), + # ) + + class Meta: + abstract = True + + +class EHerkenningConfig(AuthorizeeMixin, OpenIDConnectBaseConfig): """ - Configuration for eHerkenning authentication via OpenID connect + Configuration for eHerkenning authentication via OpenID connect. """ - identifier_claim_name = models.CharField( - _("KVK claim name"), - max_length=100, - help_text=_("The name of the claim in which the KVK of the user is stored"), - default="kvk", - ) oidc_rp_scopes_list = ArrayField( verbose_name=_("OpenID Connect scopes"), base_field=models.CharField(_("OpenID Connect scope"), max_length=50), @@ -31,7 +67,7 @@ class EHerkenningConfig(OpenIDConnectBaseConfig): blank=True, help_text=_( "OpenID Connect scopes that are requested during login. " - "These scopes are hardcoded and must be supported by the identity provider" + "These scopes are hardcoded and must be supported by the identity provider." ), ) @@ -40,27 +76,38 @@ class Meta: @property def oidcdb_username_claim(self) -> ClaimPath: - return [self.identifier_claim_name] + return self.legal_subject_claim -class EHerkenningBewindvoeringConfig(OpenIDConnectBaseConfig): - # TODO: support periods in claim keys - vertegenwoordigde_company_claim_name = models.CharField( - verbose_name=_("vertegenwoordigde company claim name"), - default="aanvrager.kvk", - max_length=50, +class EHerkenningBewindvoeringConfig(AuthorizeeMixin, OpenIDConnectBaseConfig): + # XXX: how do we determine the identifier type? + representee_claim = ClaimField( + verbose_name=_("representee identifier claim"), + # TODO: this is Anoigo, but could really be anything... + default=ClaimFieldDefault("sel_uid"), help_text=_( - "Name of the claim in which the KVK of the company being represented is stored" - ), - ) - gemachtigde_person_claim_name = models.CharField( - verbose_name=_("gemachtigde person claim name"), - default="gemachtigde.pseudoID", - max_length=50, - help_text=_( - "Name of the claim in which the ID of the person representing a company is stored" + "Name of the claim holding the identifier (like a BSN, RSIN or CoC number) " + "of the represented person/company." ), ) + + # mandate_service_id_claim = ClaimField( + # verbose_name=_("service ID claim"), + # default=ClaimFieldDefault("urn:etoegang:core:ServiceID"), + # help_text=_( + # "Name of the claim holding the service ID for which the company " + # "is authorized." + # ), + # ) + # mandate_service_uuid_claim = ClaimField( + # verbose_name=_("service UUID claim"), + # default=ClaimFieldDefault("urn:etoegang:core:ServiceUUID"), + # help_text=_( + # "Name of the claim holding the service UUID for which the company " + # "is authorized." + # ), + # ) + oidc_rp_scopes_list = ArrayField( verbose_name=_("OpenID Connect scopes"), base_field=models.CharField(_("OpenID Connect scope"), max_length=50), @@ -68,7 +115,7 @@ class EHerkenningBewindvoeringConfig(OpenIDConnectBaseConfig): blank=True, help_text=_( "OpenID Connect scopes that are requested during login. " - "These scopes are hardcoded and must be supported by the identity provider" + "These scopes are hardcoded and must be supported by the identity provider." ), ) @@ -76,13 +123,17 @@ class Meta: verbose_name = _("OpenID Connect configuration for eHerkenning Bewindvoering") @property - def digid_eherkenning_machtigen_claims(self) -> dict[str, ClaimPath]: - # TODO: this nomenclature isn't entirely correct + def mandate_claims(self) -> dict[str, ClaimPath]: return { - "vertegenwoordigde": [self.vertegenwoordigde_company_claim_name], - "gemachtigde": [self.gemachtigde_person_claim_name], + "representee": self.representee_claim, + # "authorizee_legal_subject_type": self.identifier_type_claim, + "authorizee_legal_subject": self.legal_subject_claim, + "authorizee_acting_subject": self.acting_subject_claim, + # "authorizee_branch_number": self.branch_number_claim, + # "service_id": self.mandate_service_id_claim, + # "service_uuid": self.mandate_service_uuid_claim, } @property def oidcdb_sensitive_claims(self) -> Collection[ClaimPath]: - return list(self.digid_eherkenning_machtigen_claims.values()) + return list(self.mandate_claims.values()) From c990f13fb892352214a314728c9f4a37fb2c1d30 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 12 Jun 2024 11:49:20 +0200 Subject: [PATCH 3/7] :card_file_box: Add additional fields for mandate claim information extraction From the test accounts/connections analysis, it became clear that additional information needs to be extracted from the OIDC claims. This sets up the relevant configuration fields to be able to specify which claims hold which data. --- ...onfig_mandate_service_id_claim_and_more.py | 93 +++++++++++++++++++ ...igidconfig_oidc_rp_scopes_list_and_more.py | 76 +++++++++++++++ digid_eherkenning/oidc/models/digid.py | 16 ++-- digid_eherkenning/oidc/models/eherkenning.py | 48 +++++----- 4 files changed, 201 insertions(+), 32 deletions(-) create mode 100644 digid_eherkenning/oidc/migrations/0005_digidmachtigenconfig_mandate_service_id_claim_and_more.py create mode 100644 digid_eherkenning/oidc/migrations/0006_alter_digidconfig_oidc_rp_scopes_list_and_more.py diff --git a/digid_eherkenning/oidc/migrations/0005_digidmachtigenconfig_mandate_service_id_claim_and_more.py b/digid_eherkenning/oidc/migrations/0005_digidmachtigenconfig_mandate_service_id_claim_and_more.py new file mode 100644 index 0000000..87ca213 --- /dev/null +++ b/digid_eherkenning/oidc/migrations/0005_digidmachtigenconfig_mandate_service_id_claim_and_more.py @@ -0,0 +1,93 @@ +# Generated by Django 4.2.13 on 2024-06-11 20:44 + +from django.db import migrations, models + +import mozilla_django_oidc_db.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "digid_eherkenning_oidc_generics", + "0004_migrate_config_to_claimfield", + ), + ] + + operations = [ + migrations.AddField( + model_name="digidmachtigenconfig", + name="mandate_service_id_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:nl-eid-gdi:1.0:ServiceUUID" + ), + help_text="Name of the claim holding the service UUID for which the acting subject is authorized.", + size=None, + verbose_name="service ID claim", + ), + ), + migrations.AddField( + model_name="eherkenningbewindvoeringconfig", + name="branch_number_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:etoegang:1.9:ServiceRestriction:Vestigingsnr" + ), + help_text="Name of the claim holding the value of the branch number for the authenticated company, if such a restriction applies.", + size=None, + verbose_name="branch number claim", + ), + ), + migrations.AddField( + model_name="eherkenningbewindvoeringconfig", + name="mandate_service_id_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:etoegang:core:ServiceID" + ), + help_text="Name of the claim holding the service ID for which the company is authorized.", + size=None, + verbose_name="service ID claim", + ), + ), + migrations.AddField( + model_name="eherkenningbewindvoeringconfig", + name="mandate_service_uuid_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:etoegang:core:ServiceUUID" + ), + help_text="Name of the claim holding the service UUID for which the company is authorized.", + size=None, + verbose_name="service UUID claim", + ), + ), + migrations.AddField( + model_name="eherkenningconfig", + name="branch_number_claim", + field=mozilla_django_oidc_db.fields.ClaimField( + base_field=models.CharField( + max_length=50, verbose_name="claim path segment" + ), + default=mozilla_django_oidc_db.fields.ClaimFieldDefault( + "urn:etoegang:1.9:ServiceRestriction:Vestigingsnr" + ), + help_text="Name of the claim holding the value of the branch number for the authenticated company, if such a restriction applies.", + size=None, + verbose_name="branch number claim", + ), + ), + ] diff --git a/digid_eherkenning/oidc/migrations/0006_alter_digidconfig_oidc_rp_scopes_list_and_more.py b/digid_eherkenning/oidc/migrations/0006_alter_digidconfig_oidc_rp_scopes_list_and_more.py new file mode 100644 index 0000000..6a62cd7 --- /dev/null +++ b/digid_eherkenning/oidc/migrations/0006_alter_digidconfig_oidc_rp_scopes_list_and_more.py @@ -0,0 +1,76 @@ +# Generated by Django 4.2.13 on 2024-06-12 06:50 + +from django.db import migrations, models + +import django_jsonform.models.fields + +import digid_eherkenning.oidc.models.base + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "digid_eherkenning_oidc_generics", + "0005_digidmachtigenconfig_mandate_service_id_claim_and_more", + ), + ] + + operations = [ + migrations.AlterField( + model_name="digidconfig", + name="oidc_rp_scopes_list", + field=django_jsonform.models.fields.ArrayField( + base_field=models.CharField( + max_length=50, verbose_name="OpenID Connect scope" + ), + blank=True, + default=digid_eherkenning.oidc.models.base.get_default_scopes_bsn, + help_text="OpenID Connect scopes that are requested during login. These scopes are hardcoded and must be supported by the identity provider.", + size=None, + verbose_name="OpenID Connect scopes", + ), + ), + migrations.AlterField( + model_name="digidmachtigenconfig", + name="oidc_rp_scopes_list", + field=django_jsonform.models.fields.ArrayField( + base_field=models.CharField( + max_length=50, verbose_name="OpenID Connect scope" + ), + blank=True, + default=digid_eherkenning.oidc.models.base.get_default_scopes_bsn, + help_text="OpenID Connect scopes that are requested during login. These scopes are hardcoded and must be supported by the identity provider.", + size=None, + verbose_name="OpenID Connect scopes", + ), + ), + migrations.AlterField( + model_name="eherkenningbewindvoeringconfig", + name="oidc_rp_scopes_list", + field=django_jsonform.models.fields.ArrayField( + base_field=models.CharField( + max_length=50, verbose_name="OpenID Connect scope" + ), + blank=True, + default=digid_eherkenning.oidc.models.base.get_default_scopes_bsn, + help_text="OpenID Connect scopes that are requested during login. These scopes are hardcoded and must be supported by the identity provider.", + size=None, + verbose_name="OpenID Connect scopes", + ), + ), + migrations.AlterField( + model_name="eherkenningconfig", + name="oidc_rp_scopes_list", + field=django_jsonform.models.fields.ArrayField( + base_field=models.CharField( + max_length=50, verbose_name="OpenID Connect scope" + ), + blank=True, + default=digid_eherkenning.oidc.models.base.get_default_scopes_kvk, + help_text="OpenID Connect scopes that are requested during login. These scopes are hardcoded and must be supported by the identity provider.", + size=None, + verbose_name="OpenID Connect scopes", + ), + ), + ] diff --git a/digid_eherkenning/oidc/models/digid.py b/digid_eherkenning/oidc/models/digid.py index aa93c62..9ca4fd7 100644 --- a/digid_eherkenning/oidc/models/digid.py +++ b/digid_eherkenning/oidc/models/digid.py @@ -51,14 +51,14 @@ class DigiDMachtigenConfig(OpenIDConnectBaseConfig): default=ClaimFieldDefault("urn:nl-eid-gdi:1.0:ActingSubjectID"), help_text=_("Name of the claim holding the BSN of the authorized user."), ) - # mandate_service_id_claim = ClaimField( - # verbose_name=_("service ID claim"), - # default=ClaimFieldDefault("urn:nl-eid-gdi:1.0:ServiceUUID"), - # help_text=_( - # "Name of the claim holding the service UUID for which the acting subject " - # "is authorized." - # ), - # ) + mandate_service_id_claim = ClaimField( + verbose_name=_("service ID claim"), + default=ClaimFieldDefault("urn:nl-eid-gdi:1.0:ServiceUUID"), + help_text=_( + "Name of the claim holding the service UUID for which the acting subject " + "is authorized." + ), + ) oidc_rp_scopes_list = ArrayField( verbose_name=_("OpenID Connect scopes"), diff --git a/digid_eherkenning/oidc/models/eherkenning.py b/digid_eherkenning/oidc/models/eherkenning.py index dbe9e31..6997272 100644 --- a/digid_eherkenning/oidc/models/eherkenning.py +++ b/digid_eherkenning/oidc/models/eherkenning.py @@ -42,14 +42,14 @@ class AuthorizeeMixin(models.Model): "representing the authenticated company.." ), ) - # branch_number_claim = ClaimField( - # verbose_name=_("branch number claim"), - # default=ClaimFieldDefault("urn:etoegang:1.9:ServiceRestriction:Vestigingsnr"), - # help_text=_( - # "Name of the claim holding the value of the branch number for the " - # "authenticated company, if such a restriction applies." - # ), - # ) + branch_number_claim = ClaimField( + verbose_name=_("branch number claim"), + default=ClaimFieldDefault("urn:etoegang:1.9:ServiceRestriction:Vestigingsnr"), + help_text=_( + "Name of the claim holding the value of the branch number for the " + "authenticated company, if such a restriction applies." + ), + ) class Meta: abstract = True @@ -91,22 +91,22 @@ class EHerkenningBewindvoeringConfig(AuthorizeeMixin, OpenIDConnectBaseConfig): ), ) - # mandate_service_id_claim = ClaimField( - # verbose_name=_("service ID claim"), - # default=ClaimFieldDefault("urn:etoegang:core:ServiceID"), - # help_text=_( - # "Name of the claim holding the service ID for which the company " - # "is authorized." - # ), - # ) - # mandate_service_uuid_claim = ClaimField( - # verbose_name=_("service UUID claim"), - # default=ClaimFieldDefault("urn:etoegang:core:ServiceUUID"), - # help_text=_( - # "Name of the claim holding the service UUID for which the company " - # "is authorized." - # ), - # ) + mandate_service_id_claim = ClaimField( + verbose_name=_("service ID claim"), + default=ClaimFieldDefault("urn:etoegang:core:ServiceID"), + help_text=_( + "Name of the claim holding the service ID for which the company " + "is authorized." + ), + ) + mandate_service_uuid_claim = ClaimField( + verbose_name=_("service UUID claim"), + default=ClaimFieldDefault("urn:etoegang:core:ServiceUUID"), + help_text=_( + "Name of the claim holding the service UUID for which the company " + "is authorized." + ), + ) oidc_rp_scopes_list = ArrayField( verbose_name=_("OpenID Connect scopes"), From 0860716d5a3ce02d0cc97fb92d0c567f4de54170 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 12 Jun 2024 11:50:02 +0200 Subject: [PATCH 4/7] :sparkles: Update admin form configuration Ensure that the renamed/moved and new fields are present in the admin configuration forms. --- digid_eherkenning/oidc/admin.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/digid_eherkenning/oidc/admin.py b/digid_eherkenning/oidc/admin.py index 4fbd998..d6e59fa 100644 --- a/digid_eherkenning/oidc/admin.py +++ b/digid_eherkenning/oidc/admin.py @@ -90,34 +90,47 @@ def fieldsets_factory(claim_mapping_fields: Sequence[str]): @admin.register(DigiDConfig) -class OpenIDConnectConfigDigiDAdmin(SingletonModelAdmin): +class DigiDConfigAdmin(SingletonModelAdmin): form = admin_modelform_factory(DigiDConfig) - fieldsets = fieldsets_factory(claim_mapping_fields=["identifier_claim_name"]) + fieldsets = fieldsets_factory(claim_mapping_fields=["bsn_claim"]) @admin.register(EHerkenningConfig) -class OpenIDConnectConfigEHerkenningAdmin(SingletonModelAdmin): +class EHerkenningConfigAdmin(SingletonModelAdmin): form = admin_modelform_factory(EHerkenningConfig) - fieldsets = fieldsets_factory(claim_mapping_fields=["identifier_claim_name"]) + fieldsets = fieldsets_factory( + claim_mapping_fields=[ + "identifier_type_claim", + "legal_subject_claim", + "branch_number_claim", + "acting_subject_claim", + ] + ) @admin.register(DigiDMachtigenConfig) -class OpenIDConnectConfigDigiDMachtigenAdmin(SingletonModelAdmin): +class DigiDMachtigenConfigAdmin(SingletonModelAdmin): form = admin_modelform_factory(DigiDMachtigenConfig) fieldsets = fieldsets_factory( claim_mapping_fields=[ - "vertegenwoordigde_claim_name", - "gemachtigde_claim_name", + "representee_bsn_claim", + "authorizee_bsn_claim", + "mandate_service_id_claim", ] ) @admin.register(EHerkenningBewindvoeringConfig) -class OpenIDConnectConfigEHerkenningBewindvoeringAdmin(SingletonModelAdmin): +class EHerkenningBewindvoeringConfigAdmin(SingletonModelAdmin): form = admin_modelform_factory(EHerkenningBewindvoeringConfig) fieldsets = fieldsets_factory( claim_mapping_fields=[ - "vertegenwoordigde_company_claim_name", - "gemachtigde_person_claim_name", + "representee_claim", + "identifier_type_claim", + "legal_subject_claim", + "branch_number_claim", + "acting_subject_claim", + "mandate_service_id_claim", + "mandate_service_uuid_claim", ] ) From 54206faac2599e9995b0c3509ceade4f9b96dce3 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Wed, 12 Jun 2024 18:20:20 +0200 Subject: [PATCH 5/7] :fire: Remove code for claim management This code is not directly consumed in any of our backends, and that makes testing it hard too. The consuming code is (and belongs) in Open Forms. --- digid_eherkenning/oidc/models/digid.py | 14 -------------- digid_eherkenning/oidc/models/eherkenning.py | 18 ------------------ 2 files changed, 32 deletions(-) diff --git a/digid_eherkenning/oidc/models/digid.py b/digid_eherkenning/oidc/models/digid.py index 9ca4fd7..4ccca6f 100644 --- a/digid_eherkenning/oidc/models/digid.py +++ b/digid_eherkenning/oidc/models/digid.py @@ -1,5 +1,3 @@ -from collections.abc import Collection - from django.db import models from django.utils.translation import gettext_lazy as _ @@ -73,15 +71,3 @@ class DigiDMachtigenConfig(OpenIDConnectBaseConfig): class Meta: verbose_name = _("OpenID Connect configuration for DigiD Machtigen") - - @property - def mandate_claims(self) -> dict[str, ClaimPath]: - return { - "representee": self.representee_bsn_claim, - "authorizee": self.authorizee_bsn_claim, - "service_id": self.mandate_service_id_claim, - } - - @property - def oidcdb_sensitive_claims(self) -> Collection[ClaimPath]: - return list(self.mandate_claims.values()) diff --git a/digid_eherkenning/oidc/models/eherkenning.py b/digid_eherkenning/oidc/models/eherkenning.py index 6997272..d73c2bb 100644 --- a/digid_eherkenning/oidc/models/eherkenning.py +++ b/digid_eherkenning/oidc/models/eherkenning.py @@ -1,5 +1,3 @@ -from collections.abc import Collection - from django.db import models from django.utils.translation import gettext_lazy as _ @@ -121,19 +119,3 @@ class EHerkenningBewindvoeringConfig(AuthorizeeMixin, OpenIDConnectBaseConfig): class Meta: verbose_name = _("OpenID Connect configuration for eHerkenning Bewindvoering") - - @property - def mandate_claims(self) -> dict[str, ClaimPath]: - return { - "representee": self.representee_claim, - # "authorizee_legal_subject_type": self.identifier_type_claim, - "authorizee_legal_subject": self.legal_subject_claim, - "authorizee_acting_subject": self.acting_subject_claim, - # "authorizee_branch_number": self.branch_number_claim, - # "service_id": self.mandate_service_id_claim, - # "service_uuid": self.mandate_service_uuid_claim, - } - - @property - def oidcdb_sensitive_claims(self) -> Collection[ClaimPath]: - return list(self.mandate_claims.values()) From 48667bf87a90d79175016c684765805f97bd981a Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Thu, 13 Jun 2024 14:22:44 +0200 Subject: [PATCH 6/7] :white_check_mark: Add test for claim obfuscation configuration --- tests/oidc/test_claim_obfuscation.py | 86 ++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 tests/oidc/test_claim_obfuscation.py diff --git a/tests/oidc/test_claim_obfuscation.py b/tests/oidc/test_claim_obfuscation.py new file mode 100644 index 0000000..2940c63 --- /dev/null +++ b/tests/oidc/test_claim_obfuscation.py @@ -0,0 +1,86 @@ +import pytest +from mozilla_django_oidc_db.models import OpenIDConnectConfigBase +from mozilla_django_oidc_db.typing import JSONObject +from mozilla_django_oidc_db.utils import obfuscate_claims + +from digid_eherkenning.oidc.models import ( + DigiDConfig, + DigiDMachtigenConfig, + EHerkenningBewindvoeringConfig, + EHerkenningConfig, +) + + +@pytest.mark.parametrize( + "config,claims,expected", + ( + ( + DigiDConfig(bsn_claim=["bsn"]), + {"bsn": "123456789", "other": "other"}, + {"bsn": "*******89", "other": "other"}, + ), + ( + DigiDMachtigenConfig( + representee_bsn_claim=["aanvrager"], + authorizee_bsn_claim=["gemachtigde"], + ), + { + "aanvrager": "123456789", + "gemachtigde": "123456789", + "other": "other", + }, + { + "aanvrager": "*******89", + "gemachtigde": "*******89", + "other": "other", + }, + ), + ( + EHerkenningConfig( + legal_subject_claim=["kvk"], + acting_subject_claim=["ActingSubject"], + branch_number_claim=["branch"], + ), + { + "kvk": "12345678", + "branch": "112233445566", + # this is already obfuscated by the broker + "ActingSubject": "1234567890@0987654321", + }, + { + "kvk": "*******8", + "branch": "**********66", + # this is already obfuscated by the broker + "ActingSubject": "1234567890@0987654321", + }, + ), + ( + EHerkenningBewindvoeringConfig( + representee_claim=["bsn"], + legal_subject_claim=["kvk"], + acting_subject_claim=["ActingSubject"], + branch_number_claim=["branch"], + ), + { + "bsn": "123456789", + "kvk": "12345678", + "branch": "112233445566", + # this is already obfuscated by the broker + "ActingSubject": "1234567890@0987654321", + }, + { + "bsn": "*******89", + "kvk": "*******8", + "branch": "**********66", + # this is already obfuscated by the broker + "ActingSubject": "1234567890@0987654321", + }, + ), + ), +) +def test_claim_obfuscation( + config: OpenIDConnectConfigBase, claims: JSONObject, expected: JSONObject +): + obfuscated = obfuscate_claims(claims, config.oidcdb_sensitive_claims) + + assert obfuscated == expected From 04242abbfbc429913175eb672c5437a25365f8b9 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Thu, 13 Jun 2024 14:23:24 +0200 Subject: [PATCH 7/7] :sparkles: Define claim obfuscation parameters on config models Downstream projects will miss/forget this, and we know which claims hold privacy-sensitive information. --- .../0004_migrate_config_to_claimfield.py | 2 +- digid_eherkenning/oidc/models/digid.py | 9 +++++++ digid_eherkenning/oidc/models/eherkenning.py | 24 +++++++++++++++---- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/digid_eherkenning/oidc/migrations/0004_migrate_config_to_claimfield.py b/digid_eherkenning/oidc/migrations/0004_migrate_config_to_claimfield.py index b4748d0..4f9700e 100644 --- a/digid_eherkenning/oidc/migrations/0004_migrate_config_to_claimfield.py +++ b/digid_eherkenning/oidc/migrations/0004_migrate_config_to_claimfield.py @@ -140,7 +140,7 @@ class Migration(migrations.Migration): max_length=50, verbose_name="claim path segment" ), default=mozilla_django_oidc_db.fields.ClaimFieldDefault("sel_uid"), - help_text="Name of the claim holding the identifier (like a BSN, RSIN or CoC number) of the represented person/company.", + help_text="Name of the claim holding the BSN of the represented person.", size=None, verbose_name="representee identifier claim", ), diff --git a/digid_eherkenning/oidc/models/digid.py b/digid_eherkenning/oidc/models/digid.py index 4ccca6f..b7b676e 100644 --- a/digid_eherkenning/oidc/models/digid.py +++ b/digid_eherkenning/oidc/models/digid.py @@ -1,3 +1,5 @@ +from collections.abc import Sequence + from django.db import models from django.utils.translation import gettext_lazy as _ @@ -71,3 +73,10 @@ class DigiDMachtigenConfig(OpenIDConnectBaseConfig): class Meta: verbose_name = _("OpenID Connect configuration for DigiD Machtigen") + + @property + def oidcdb_sensitive_claims(self) -> Sequence[ClaimPath]: + return [ + self.representee_bsn_claim, # type: ignore + self.authorizee_bsn_claim, # type: ignore + ] diff --git a/digid_eherkenning/oidc/models/eherkenning.py b/digid_eherkenning/oidc/models/eherkenning.py index d73c2bb..2dd2c15 100644 --- a/digid_eherkenning/oidc/models/eherkenning.py +++ b/digid_eherkenning/oidc/models/eherkenning.py @@ -1,3 +1,5 @@ +from collections.abc import Sequence + from django.db import models from django.utils.translation import gettext_lazy as _ @@ -52,6 +54,13 @@ class AuthorizeeMixin(models.Model): class Meta: abstract = True + @property + def oidcdb_sensitive_claims(self) -> Sequence[ClaimPath]: + return [ + self.legal_subject_claim, # type: ignore + self.branch_number_claim, # type: ignore + ] + class EHerkenningConfig(AuthorizeeMixin, OpenIDConnectBaseConfig): """ @@ -78,15 +87,13 @@ def oidcdb_username_claim(self) -> ClaimPath: class EHerkenningBewindvoeringConfig(AuthorizeeMixin, OpenIDConnectBaseConfig): - # XXX: how do we determine the identifier type? + # NOTE: Discussion with an employee from Anoigo states this will always be a BSN, + # not an RSIN or CoC number. representee_claim = ClaimField( verbose_name=_("representee identifier claim"), # TODO: this is Anoigo, but could really be anything... default=ClaimFieldDefault("sel_uid"), - help_text=_( - "Name of the claim holding the identifier (like a BSN, RSIN or CoC number) " - "of the represented person/company." - ), + help_text=_("Name of the claim holding the BSN of the represented person."), ) mandate_service_id_claim = ClaimField( @@ -119,3 +126,10 @@ class EHerkenningBewindvoeringConfig(AuthorizeeMixin, OpenIDConnectBaseConfig): class Meta: verbose_name = _("OpenID Connect configuration for eHerkenning Bewindvoering") + + @property + def oidcdb_sensitive_claims(self) -> Sequence[ClaimPath]: + base = super().oidcdb_sensitive_claims + return base + [ + self.representee_claim, # type: ignore + ]