From 8f48703f54c84a9cedf2eb8454e45cb9946b8426 Mon Sep 17 00:00:00 2001 From: Sergei Maertens Date: Thu, 12 Dec 2024 18:18:30 +0100 Subject: [PATCH] :boom: [#3283] Remove legacy machtigen context The shape of data that we store was badly documented, not in the least because we barely had any idea what kind of information we receive. Since Open Forms 2.7.0 it has been fleshed out what the mandata context looks like, and properly implemented backed by a proposal for a standard which is implemented all the way to Open Zaak to try out. This legacy context is now being removed in favour of the new way to track and expose the mandata context. --- docs/installation/upgrade-300.rst | 15 ++++++ docs/manual/forms/variables.rst | 12 +++-- src/openforms/authentication/admin.py | 6 +-- .../contrib/digid_eherkenning_oidc/plugin.py | 8 --- .../tests/test_auth_context_data.py | 22 +------- .../0004_remove_authinfo_machtigen.py | 20 ++++++++ src/openforms/authentication/models.py | 10 ---- src/openforms/authentication/signals.py | 1 - .../static_variables/static_variables.py | 2 - .../authentication/tests/factories.py | 1 - .../authentication/tests/test_signals.py | 25 --------- .../tests/test_static_variables.py | 2 - src/openforms/authentication/typing.py | 3 -- src/openforms/conf/base.py | 2 +- .../contrib/haalcentraal_brp/plugin.py | 5 +- .../haalcentraal_brp/tests/test_plugin.py | 51 ------------------- .../prefill/contrib/stufbg/plugin.py | 5 +- .../contrib/stufbg/tests/test_plugin.py | 35 ------------- 18 files changed, 51 insertions(+), 174 deletions(-) create mode 100644 src/openforms/authentication/migrations/0004_remove_authinfo_machtigen.py diff --git a/docs/installation/upgrade-300.rst b/docs/installation/upgrade-300.rst index 02f3b24ad7..56ea34ff78 100644 --- a/docs/installation/upgrade-300.rst +++ b/docs/installation/upgrade-300.rst @@ -32,6 +32,21 @@ in the error monitoring if that's set up correctly. .. note:: Existing, automatically converted forms are crash-free because we create an explicit fallback logic rule that mimicks the old behaviour. +Removed legacy "machtigen" context +================================== + +Open Forms 2.7.0 revamped how mandates ("machtigingen") are captured when DigiD +Machtigen or eHerkenning Bewindvoering are used. We kept the old structure in place to +give some time to transition and are now removing this in favour of the new setup. For +the time being, the new setup still defaults to "Lax" mode, meaning it will continue to +work when some information is missing that is considered required. We recommend to opt +in to strict mode though, through the ``DIGID_EHERKENNING_OIDC_STRICT`` feature +flag/environment variable. + +You are affected by this if you use the static variable ``auth.machtigen`` in your +registration backends. Instead, you should use the ``auth_context`` static variable. +Since Open Forms 2.7.0, the same data has been saved to both datastructures. + Form components/fields changes ============================== diff --git a/docs/manual/forms/variables.rst b/docs/manual/forms/variables.rst index 5d7c58989f..c6345ae00e 100644 --- a/docs/manual/forms/variables.rst +++ b/docs/manual/forms/variables.rst @@ -62,12 +62,11 @@ auth_context object ``{"source": "...", ...}`` De volledige authentica **Verouderde variabelen** Deze variabelen zijn nog wel beschikbaar, maar we raden aan om deze niet meer te -gebruiken. In versie 3.0 van Open Formulieren kunnen deze verwijderd worden. +gebruiken. In versie 4.0 van Open Formulieren kunnen deze verwijderd worden. =============== ========= =========================== ========================================================================= Variabele Type Voorbeeldwaarde Toelichting =============== ========= =========================== ========================================================================= -auth.machtigen object Verouderd, wordt in 3.0 verwijderd. auth.attribute string ``bsn`` Kan de waarden ``bsn``, ``kvk`` of ``pseudo`` hebben (verouderd, gebruik bij voorkeur ``auth_type``). =============== ========= =========================== ========================================================================= @@ -87,14 +86,17 @@ Wanneer er niet ingelogd is op het formulier, dan is de waarde van deze variabel .. note:: De ``auth_context`` variabele gaat op termijn de ``auth`` variabele vervangen, - maar voorlopig wordt deze laatste niet verwijderd. We verwachten in Open Formulieren - 3.0 enkel de ``auth.machtigen`` variabele te verwijderen omdat de structuur - hiervan altijd vaag en onbetrouwbaar was. + maar voorlopig wordt deze laatste niet verwijderd. Tip: in plaats van ``auth.plugin`` kan je beter ``auth_context_source`` of ``auth_type`` gebruiken - de eerste is minder flexibel/uitwisselbaar, terwijl de tweede wel goed de semantische betekenis bevat of het om een burger of bedrijf gaat. +.. versionremoved:: 3.0 + + De ``auth.machtigen`` variabele is verwijderd omdat de structuur hiervan vaag en + onbetrouwbaar was. Gebruik ``auth_context`` in de plaats. + De variabele bevat een bak aan informatie, gestructureerd volgens het authenticatiecontextdatamodel_. De structuur is als volgt: diff --git a/src/openforms/authentication/admin.py b/src/openforms/authentication/admin.py index 78d31ff2ce..4803ebcbc1 100644 --- a/src/openforms/authentication/admin.py +++ b/src/openforms/authentication/admin.py @@ -20,11 +20,9 @@ class Meta: "legal_subject_identifier_value", "legal_subject_service_restriction", "mandate_context", - "machtigen", "attribute_hashed", ) widgets = { - "machtigen": forms.Textarea(attrs={"cols": 20, "rows": 1}), "plugin": forms.TextInput(attrs={"size": 20}), "value": forms.TextInput(attrs={"size": 20}), } @@ -81,7 +79,7 @@ class AuthInfoInline(admin.StackedInline): ( _("Mandate"), { - "fields": ("mandate_context", "machtigen"), + "fields": ("mandate_context",), "classes": ("collapse",), }, ), @@ -137,7 +135,7 @@ class AuthInfoAdmin(admin.ModelAdmin): ) }, ), - (_("Mandate"), {"fields": ("mandate_context", "machtigen")}), + (_("Mandate"), {"fields": ("mandate_context",)}), ( _("Misc"), {"fields": ("attribute_hashed",), "classes": ("collapse in",)}, diff --git a/src/openforms/authentication/contrib/digid_eherkenning_oidc/plugin.py b/src/openforms/authentication/contrib/digid_eherkenning_oidc/plugin.py index 7971b40bc3..28dc7e3ba8 100644 --- a/src/openforms/authentication/contrib/digid_eherkenning_oidc/plugin.py +++ b/src/openforms/authentication/contrib/digid_eherkenning_oidc/plugin.py @@ -281,10 +281,6 @@ def transform_claims(self, normalized_claims: DigiDmachtigenClaims) -> FormAuth: "legal_subject_identifier_type": "bsn", "legal_subject_identifier_value": authorizee, "mandate_context": mandate_context, - # DeprecationWarning - legacy, remove in 3.0 - "machtigen": { - "identifier_value": authorizee, - }, } def get_label(self) -> str: @@ -375,10 +371,6 @@ def transform_claims(self, normalized_claims: EHBewindvoeringClaims) -> FormAuth "role": "bewindvoerder", "services": services, }, - # DeprecationWarning - legacy, remove in 3.0 - "machtigen": { - "identifier_value": authorizee, - }, } if service_restriction := normalized_claims.get("branch_number_claim", ""): diff --git a/src/openforms/authentication/contrib/digid_eherkenning_oidc/tests/test_auth_context_data.py b/src/openforms/authentication/contrib/digid_eherkenning_oidc/tests/test_auth_context_data.py index 55fe233ef9..e2e6357ccf 100644 --- a/src/openforms/authentication/contrib/digid_eherkenning_oidc/tests/test_auth_context_data.py +++ b/src/openforms/authentication/contrib/digid_eherkenning_oidc/tests/test_auth_context_data.py @@ -178,10 +178,6 @@ def test_record_auth_context(self): auth_context["authorizee"]["legalSubject"], {"identifierType": "bsn", "identifier": "999999999"}, ) - # TODO: remove in Open Forms 3.0 - with self.subTest("legacy structure"): - machtigen = submission.auth_info.machtigen - self.assertEqual(machtigen, {"identifier_value": "999999999"}) @mock_digid_machtigen_config(mandate_service_id_claim=["required-but-absent-claim"]) def test_new_required_claims_are_backwards_compatible(self): @@ -195,7 +191,7 @@ def test_new_required_claims_are_backwards_compatible(self): runtime implications. """ warnings.warn( - "Legacy behaviour will be removed in Open Forms 3.0", DeprecationWarning + "Legacy behaviour will be removed in Open Forms 4.0", DeprecationWarning ) self._login_and_start_form( "digid_machtigen_oidc", @@ -218,10 +214,6 @@ def test_new_required_claims_are_backwards_compatible(self): auth_context["authorizee"]["legalSubject"], {"identifierType": "bsn", "identifier": "999999999"}, ) - # TODO: remove in Open Forms 3.0 - with self.subTest("legacy structure"): - machtigen = submission.auth_info.machtigen - self.assertEqual(machtigen, {"identifier_value": "999999999"}) @override_settings(ALLOWED_HOSTS=["*"]) @@ -274,11 +266,6 @@ def test_record_auth_context(self): }, ) - # TODO: remove in Open Forms 3.0 - with self.subTest("legacy structure"): - machtigen = submission.auth_info.machtigen - self.assertEqual(machtigen, {"identifier_value": "12345678"}) - @mock_eherkenning_bewindvoering_config( mandate_service_id_claim=["required-but-absent-claim1"], mandate_service_uuid_claim=["required-but-absent-claim2"], @@ -294,7 +281,7 @@ def test_new_required_claims_are_backwards_compatible(self): runtime implications. """ warnings.warn( - "Legacy behaviour will be removed in Open Forms 3.0", DeprecationWarning + "Legacy behaviour will be removed in Open Forms 4.0", DeprecationWarning ) self._login_and_start_form( "eherkenning_bewindvoering_oidc", @@ -327,11 +314,6 @@ def test_new_required_claims_are_backwards_compatible(self): {"role": "bewindvoerder", "services": []}, ) - # TODO: remove in Open Forms 3.0 - with self.subTest("legacy structure"): - machtigen = submission.auth_info.machtigen - self.assertEqual(machtigen, {"identifier_value": "12345678"}) - @mock_eherkenning_bewindvoering_config(branch_number_claim=["vestiging"]) def test_record_vestiging_restriction(self): self._login_and_start_form( diff --git a/src/openforms/authentication/migrations/0004_remove_authinfo_machtigen.py b/src/openforms/authentication/migrations/0004_remove_authinfo_machtigen.py new file mode 100644 index 0000000000..f29261d30d --- /dev/null +++ b/src/openforms/authentication/migrations/0004_remove_authinfo_machtigen.py @@ -0,0 +1,20 @@ +# Generated by Django 4.2.17 on 2024-12-12 16:54 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "of_authentication", + "0003_authinfo_legal_subject_service_restriction_and_more", + ), + ] + + operations = [ + migrations.RemoveField( + model_name="authinfo", + name="machtigen", + ), + ] diff --git a/src/openforms/authentication/models.py b/src/openforms/authentication/models.py index 01caec5270..940bb467b1 100644 --- a/src/openforms/authentication/models.py +++ b/src/openforms/authentication/models.py @@ -208,16 +208,6 @@ class AuthInfo(BaseAuthInfo): null=True, ) - # deprecated! - machtigen = models.JSONField( - verbose_name=_("machtigen"), - help_text=_( - "Data related to any 'machtiging' (authorising someone else to perform actions on your behalf)." - ), - blank=True, - null=True, - ) - identifying_attributes = BaseAuthInfo.identifying_attributes + ( "acting_subject_identifier_value", "legal_subject_identifier_value", diff --git a/src/openforms/authentication/signals.py b/src/openforms/authentication/signals.py index 4ed171726c..7d8cb2b163 100644 --- a/src/openforms/authentication/signals.py +++ b/src/openforms/authentication/signals.py @@ -126,7 +126,6 @@ def set_auth_attribute_on_session( "attribute": form_auth["attribute"], "plugin": form_auth["plugin"], } - # TODO the "machtigingen" key disappeared store_auth_details(instance, auth_save) store_registrator_details(instance, registrator_save) else: diff --git a/src/openforms/authentication/static_variables/static_variables.py b/src/openforms/authentication/static_variables/static_variables.py index f9d9fa9b4d..cd8f07d5e0 100644 --- a/src/openforms/authentication/static_variables/static_variables.py +++ b/src/openforms/authentication/static_variables/static_variables.py @@ -48,8 +48,6 @@ def get_initial_value( plugin=submission.auth_info.plugin, attribute=submission.auth_info.attribute, value=submission.auth_info.value, - # DeprecationWarning -> delete machtigen in Open Forms 3.0 - machtigen=submission.auth_info.machtigen, ) return auth_data diff --git a/src/openforms/authentication/tests/factories.py b/src/openforms/authentication/tests/factories.py index c25d3b89ba..03ba5f1fb8 100644 --- a/src/openforms/authentication/tests/factories.py +++ b/src/openforms/authentication/tests/factories.py @@ -14,7 +14,6 @@ class AuthInfoFactory(factory.django.DjangoModelFactory): plugin = "digid" attribute = AuthAttribute.bsn value = "123456782" - machtigen = {} submission = factory.SubFactory( "openforms.submissions.tests.factories.SubmissionFactory" ) diff --git a/src/openforms/authentication/tests/test_signals.py b/src/openforms/authentication/tests/test_signals.py index c5c1fd905b..b87a83dcec 100644 --- a/src/openforms/authentication/tests/test_signals.py +++ b/src/openforms/authentication/tests/test_signals.py @@ -279,31 +279,6 @@ def test_setting_auth_attributes_flips_hashed_flag(self): self.assertEqual(submission.auth_info.value, "123456789") self.assertFalse(submission.auth_info.attribute_hashed) - def test_auth_with_legacy_digid_machtigen(self): - submission = SubmissionFactory.create( - form__authentication_backends=["digid_machtigen_oidc"] - ) - user = UserFactory() - request = factory.get("/foo") - request.user = user - request.session = { - FORM_AUTH_SESSION_KEY: { - "plugin": "digid_machtigen_oidc", - "attribute": "bsn", - "value": "123123123", - "machtigen": {"identifier_value": "123456782"}, - } - } - - set_auth_attribute_on_session(sender=None, instance=submission, request=request) - - submission.refresh_from_db() - - self.assertTrue(submission.is_authenticated) - self.assertEqual( - submission.auth_info.machtigen["identifier_value"], "123456782" - ) - @freeze_time("2021-11-26T17:00:00+00:00") class SetCosignDataTests(APITestCase): diff --git a/src/openforms/authentication/tests/test_static_variables.py b/src/openforms/authentication/tests/test_static_variables.py index f6020c5616..271a3ecd08 100644 --- a/src/openforms/authentication/tests/test_static_variables.py +++ b/src/openforms/authentication/tests/test_static_variables.py @@ -14,7 +14,6 @@ def test_auth_static_data(self): plugin="test-plugin", attribute=AuthAttribute.bsn, value="111222333", - machtigen={AuthAttribute.bsn: "123456789"}, ) static_data = { @@ -27,7 +26,6 @@ def test_auth_static_data(self): "plugin": "test-plugin", "attribute": AuthAttribute.bsn, "value": "111222333", - "machtigen": {AuthAttribute.bsn: "123456789"}, }, "auth_bsn": "111222333", "auth_kvk": "", diff --git a/src/openforms/authentication/typing.py b/src/openforms/authentication/typing.py index 0cb0ee7fab..0d7a050ccc 100644 --- a/src/openforms/authentication/typing.py +++ b/src/openforms/authentication/typing.py @@ -23,6 +23,3 @@ class FormAuth(BaseAuth): legal_subject_identifier_value: NotRequired[str] legal_subject_service_restriction: NotRequired[str] mandate_context: NotRequired[JSONObject] - - # deprecated - machtigen: NotRequired[dict | None] diff --git a/src/openforms/conf/base.py b/src/openforms/conf/base.py index 735f2b8109..1ee3cfbff6 100644 --- a/src/openforms/conf/base.py +++ b/src/openforms/conf/base.py @@ -1193,7 +1193,7 @@ { "condition": "boolean", # DeprecationWarning - # TODO: set default to `True` in Open Forms 3.0 + # TODO: set default to `True` in Open Forms 4.0 "value": config("DIGID_EHERKENNING_OIDC_STRICT", default=False), } ], diff --git a/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py b/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py index 43ae3a9d7d..5492cba531 100644 --- a/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py +++ b/src/openforms/prefill/contrib/haalcentraal_brp/plugin.py @@ -89,7 +89,6 @@ def get_identifier_value( return submission.auth_info.value if identifier_role == IdentifierRoles.authorizee: - legacy_fallback = submission.auth_info.machtigen.get("identifier_value") auth_context = submission.auth_info.to_auth_context_data() # check if we have new-style authentication context capturing, and favour # that over the legacy format @@ -99,8 +98,8 @@ def get_identifier_value( "representee" not in auth_context or legal_subject["identifierType"] != "bsn" ): - return legacy_fallback - return legal_subject["identifier"] or legacy_fallback + return None + return legal_subject["identifier"] @classmethod def get_prefill_values( diff --git a/src/openforms/prefill/contrib/haalcentraal_brp/tests/test_plugin.py b/src/openforms/prefill/contrib/haalcentraal_brp/tests/test_plugin.py index 3b1c6c1a85..2ffd810e18 100644 --- a/src/openforms/prefill/contrib/haalcentraal_brp/tests/test_plugin.py +++ b/src/openforms/prefill/contrib/haalcentraal_brp/tests/test_plugin.py @@ -134,36 +134,12 @@ def test_prefill_values_not_authenticated(self): self.assertEqual(values, {}) # type: ignore - def test_prefill_values_for_gemachtigde_legacy_format(self): - Attributes = get_attributes_cls() - submission = SubmissionFactory.create( - auth_info__value="111111111", - auth_info__machtigen={"identifier_value": "999990676"}, - ) - assert submission.is_authenticated - plugin = HaalCentraalPrefill(PLUGIN_IDENTIFIER) - - values = plugin.get_prefill_values( - submission, - attributes=[Attributes.naam_voornamen, Attributes.naam_geslachtsnaam], - identifier_role=IdentifierRoles.authorizee, - ) - - self.assertEqual( - values, - { - "naam.voornamen": "Cornelia Francisca", - "naam.geslachtsnaam": "Wiegman", - }, - ) # type: ignore - def test_prefill_values_for_gemachtigde_by_bsn(self): Attributes = get_attributes_cls() submission = SubmissionFactory.create( auth_info__value="111111111", auth_info__is_digid_machtigen=True, auth_info__legal_subject_identifier_value="999990676", - auth_info__machtigen={}, # make sure legacy format is empty ) assert submission.is_authenticated plugin = HaalCentraalPrefill(PLUGIN_IDENTIFIER) @@ -236,17 +212,6 @@ def test_extract_authorizee_identifier_value(self): ), None, ), - # legacy fallback - ( - SubmissionFactory.create( - auth_info__is_digid_machtigen=True, - auth_info__legal_subject_identifier_type="", - auth_info__legal_subject_identifier_value="", - auth_info__machtigen={"identifier_value": "999333666"}, - auth_info__mandate_context=None, - ), - "999333666", - ), ) plugin = HaalCentraalPrefill(PLUGIN_IDENTIFIER) @@ -277,14 +242,6 @@ def test_person_not_found_returns_empty(self): ) super().test_person_not_found_returns_empty() - def test_prefill_values_for_gemachtigde_legacy_format(self): - self.requests_mock.get( - "https://personen/api/ingeschrevenpersonen/999990676", - status_code=200, - json=load_json_mock("ingeschrevenpersonen.v1-full.json"), - ) - super().test_prefill_values_for_gemachtigde_legacy_format() - def test_prefill_values_for_gemachtigde_by_bsn(self): self.requests_mock.get( "https://personen/api/ingeschrevenpersonen/999990676", @@ -321,14 +278,6 @@ def test_person_not_found_returns_empty(self): ) super().test_person_not_found_returns_empty() - def test_prefill_values_for_gemachtigde_legacy_format(self): - self.requests_mock.post( - "https://personen/api/personen", - status_code=200, - json=load_json_mock("ingeschrevenpersonen.v2-full.json"), - ) - super().test_prefill_values_for_gemachtigde_legacy_format() - def test_prefill_values_for_gemachtigde_by_bsn(self): self.requests_mock.post( "https://personen/api/personen", diff --git a/src/openforms/prefill/contrib/stufbg/plugin.py b/src/openforms/prefill/contrib/stufbg/plugin.py index d6081cc2af..68c7cfb5dd 100644 --- a/src/openforms/prefill/contrib/stufbg/plugin.py +++ b/src/openforms/prefill/contrib/stufbg/plugin.py @@ -143,7 +143,6 @@ def get_identifier_value( return submission.auth_info.value if identifier_role == IdentifierRoles.authorizee: - legacy_fallback = submission.auth_info.machtigen.get("identifier_value") auth_context = submission.auth_info.to_auth_context_data() # check if we have new-style authentication context capturing, and favour # that over the legacy format @@ -153,8 +152,8 @@ def get_identifier_value( "representee" not in auth_context or legal_subject["identifierType"] != "bsn" ): - return legacy_fallback - return legal_subject["identifier"] or legacy_fallback + return None + return legal_subject["identifier"] @classmethod def get_prefill_values( diff --git a/src/openforms/prefill/contrib/stufbg/tests/test_plugin.py b/src/openforms/prefill/contrib/stufbg/tests/test_plugin.py index 5e2326c4a1..f949c4c820 100644 --- a/src/openforms/prefill/contrib/stufbg/tests/test_plugin.py +++ b/src/openforms/prefill/contrib/stufbg/tests/test_plugin.py @@ -219,29 +219,6 @@ def test_prefill_values_not_authenticated(self): self.assertEqual(values, {}) - def test_get_available_attributes_for_gemachtigde_legacy_format(self): - client_patcher = mock_stufbg_client("StufBgResponse.xml") - self.addCleanup(client_patcher.stop) - attributes = [c.value for c in FieldChoices] - submission = SubmissionFactory.create( - auth_info__value="111111111", - auth_info__machtigen={"identifier_value": "999992314"}, - ) - - values = self.plugin.get_prefill_values( - submission, attributes, IdentifierRoles.authorizee - ) - - self.assertEqual(values["bsn"], "999992314") - self.assertEqual(values["voornamen"], "Media") - self.assertEqual(values["geslachtsnaam"], "Maykin") - self.assertEqual(values["straatnaam"], "Keizersgracht") - self.assertEqual(values["huisnummer"], "117") - self.assertEqual(values["huisletter"], "A") - self.assertEqual(values["huisnummertoevoeging"], "B") - self.assertEqual(values["postcode"], "1015 CJ") - self.assertEqual(values["woonplaatsNaam"], "Amsterdam") - def test_prefill_values_for_gemachtigde_by_bsn(self): client_patcher = mock_stufbg_client("StufBgResponse.xml") self.addCleanup(client_patcher.stop) @@ -250,7 +227,6 @@ def test_prefill_values_for_gemachtigde_by_bsn(self): auth_info__value="111111111", auth_info__is_digid_machtigen=True, auth_info__legal_subject_identifier_value="999990676", - auth_info__machtigen={}, # make sure legacy format is empty ) values = self.plugin.get_prefill_values( @@ -285,17 +261,6 @@ def test_extract_authorizee_identifier_value(self): ), None, ), - # legacy fallback - ( - SubmissionFactory.create( - auth_info__is_digid_machtigen=True, - auth_info__legal_subject_identifier_type="", - auth_info__legal_subject_identifier_value="", - auth_info__machtigen={"identifier_value": "999333666"}, - auth_info__mandate_context=None, - ), - "999333666", - ), ) for submission, expected in cases: with self.subTest(auth_context=submission.auth_info.to_auth_context_data()):