From c1b56b8b7d5e9478b1fe0fc90050152b67beec67 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Mon, 11 Nov 2024 14:59:32 +0000 Subject: [PATCH 01/12] use navet in dev environment --- src/eduid/workers/msg/tasks.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/eduid/workers/msg/tasks.py b/src/eduid/workers/msg/tasks.py index 5aa9c977b..3608475ba 100644 --- a/src/eduid/workers/msg/tasks.py +++ b/src/eduid/workers/msg/tasks.py @@ -153,10 +153,6 @@ def get_postal_address(self, identity_number: str) -> OrderedDict[str, Any] | No :param identity_number: Swedish national identity number :return: dict containing name and postal address """ - # Only log the message if devel_mode is enabled - if MsgCelerySingleton.worker_config.devel_mode is True: - return self.get_devel_postal_address() - data = self._get_navet_data(identity_number) # Filter name and address from the Navet lookup results return navet_get_name_and_official_address(data) @@ -187,10 +183,6 @@ def get_relations(self, identity_number: str) -> OrderedDict[str, Any] | None: :param identity_number: Swedish national identity number :return: dict containing name and postal address """ - # Only log the message if devel_mode is enabled - if MsgCelerySingleton.worker_config.devel_mode is True: - return self.get_devel_relations() - data = self._get_navet_data(identity_number) # Filter relations from the Navet lookup results return navet_get_relations(data) @@ -229,10 +221,6 @@ def get_devel_relations() -> OrderedDict[str, Any]: return result def get_all_navet_data(self, identity_number: str) -> OrderedDict[str, Any] | None: - # Only log the message if devel_mode is enabled - if MsgCelerySingleton.worker_config.devel_mode is True: - return self.get_devel_all_navet_data(identity_number) - data = self._get_navet_data(identity_number) return navet_get_all_data(data) From ccfd08f74fde0afddbb0e572de8fcee5446c311a Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Tue, 19 Nov 2024 14:39:29 +0000 Subject: [PATCH 02/12] get reference national identity number --- src/eduid/webapp/common/api/utils.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/eduid/webapp/common/api/utils.py b/src/eduid/webapp/common/api/utils.py index 921361871..22e7b170a 100644 --- a/src/eduid/webapp/common/api/utils.py +++ b/src/eduid/webapp/common/api/utils.py @@ -13,6 +13,7 @@ from eduid.common.config.base import EduIDBaseAppConfig, Pysaml2SPConfigMixin from eduid.common.misc.timeutil import utc_now +from eduid.common.rpc.msg_relay import MsgRelay from eduid.userdb import User, UserDB from eduid.userdb.exceptions import MultipleUsersReturned, UserDBValueError, UserDoesNotExist from eduid.webapp.common.api.exceptions import ApiException @@ -294,3 +295,16 @@ def is_throttled(ts: datetime, min_wait: timedelta) -> bool: def is_expired(ts: datetime, max_age: timedelta) -> bool: return utc_now() - ts > max_age + + +def get_reference_nin_from_navet_data(nin: str) -> str | None: + """ + Check if the NIN has changed in Navet data. + """ + msg_relay = get_from_current_app("msg_relay", MsgRelay) + + navet_data = msg_relay.get_all_navet_data(nin=nin) + if navet_data.person.reference_national_identity_number: + return navet_data.person.reference_national_identity_number + else: + return None From 61b2cf4b598d9de6236fd6df40d97340a7585d6c Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Tue, 19 Nov 2024 14:41:45 +0000 Subject: [PATCH 03/12] allow verification and replacement of locked nin if reference nin is the same as locked nin let verification through and replace the locked nin with the new verified nin --- src/eduid/webapp/common/api/helpers.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/eduid/webapp/common/api/helpers.py b/src/eduid/webapp/common/api/helpers.py index bc89963a6..a372771df 100644 --- a/src/eduid/webapp/common/api/helpers.py +++ b/src/eduid/webapp/common/api/helpers.py @@ -28,7 +28,7 @@ from eduid.userdb.user import TUserSubclass, User from eduid.userdb.userdb import UserDB from eduid.webapp.common.api.app import EduIDBaseApp -from eduid.webapp.common.api.utils import get_from_current_app, save_and_sync_user +from eduid.webapp.common.api.utils import get_from_current_app, get_reference_nin_from_navet_data, save_and_sync_user __author__ = "lundberg" @@ -235,6 +235,10 @@ def verify_nin_for_user( current_app.logger.debug(f"NIN: {proofing_user.identities.nin.number}") return True + reference_nin = get_reference_nin_from_navet_data(proofing_state.nin.number) + if reference_nin is not None: + current_app.logger.debug(f"verified nin has reference_nin: {reference_nin}") + # check if the users current nin is the same as the one just verified # if there is no locked nin identity or the locked nin identity matches we can replace the current nin identity # with the one just verified @@ -244,8 +248,9 @@ def verify_nin_for_user( if ( proofing_user.locked_identity.nin is not None and proofing_user.locked_identity.nin.number != proofing_state.nin.number + and proofing_user.locked_identity.nin.number != reference_nin ): - raise LockedIdentityViolation("users locked nin does not match verified nin") + raise LockedIdentityViolation("users locked nin does not match verified nin or reference nin") # user has no locked nin identity or the user has previously verified the nin # replace the never verified nin with the one just verified @@ -268,6 +273,14 @@ def verify_nin_for_user( proofing_user.identities.nin.proofing_method = proofing_method proofing_user.identities.nin.proofing_version = proofing_log_entry.proofing_version + # Replace locked nin with verified nin if it has changed in the population registry + if ( + reference_nin is not None + and proofing_user.locked_identity.nin is not None + and proofing_user.locked_identity.nin.number == reference_nin + ): + proofing_user.locked_identity.replace(element=nin_identity) + # Update users name proofing_user = set_user_names_from_nin_proofing(user=proofing_user, proofing_log_entry=proofing_log_entry) From 446998686a1612dadffd830d411efab4c4fc870c Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Tue, 19 Nov 2024 14:43:58 +0000 Subject: [PATCH 04/12] let views through if reference nin is same as locked nin --- src/eduid/webapp/common/api/decorators.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/eduid/webapp/common/api/decorators.py b/src/eduid/webapp/common/api/decorators.py index 60a97d7cb..48c6c3f5f 100644 --- a/src/eduid/webapp/common/api/decorators.py +++ b/src/eduid/webapp/common/api/decorators.py @@ -19,8 +19,11 @@ FluxResponseStatus, FluxSuccessResponse, ) -from eduid.webapp.common.api.utils import get_user +from eduid.webapp.common.api.utils import get_reference_nin_from_navet_data, get_user from eduid.webapp.common.session import session +from eduid.webapp.letter_proofing.app import LetterProofingApp +from eduid.webapp.lookup_mobile_proofing.app import MobileProofingApp +from eduid.webapp.oidc_proofing.app import OIDCProofingApp __author__ = "lundberg" @@ -116,6 +119,16 @@ def verify_identity_decorator(*args: Any, **kwargs: Any) -> EduidViewReturnType: if isinstance(locked_nin, NinIdentity) and locked_nin.number != kwargs["nin"]: logger.info("User has a different locked NIN") logger.debug(f"Locked NIN: {locked_nin.number}. New NIN: {kwargs['nin']}") + if isinstance(session.app, MobileProofingApp | LetterProofingApp | OIDCProofingApp): + ref = get_reference_nin_from_navet_data(kwargs["nin"]) + logger.debug(f"New NIN has reference NIN: {ref}") + # If the reference NIN is the same as the locked NIN, we can continue with the verification + if locked_nin.number == ref: + logger.info( + "User has a different locked NIN but it is the same as the reference NIN for the new NIN" + ) + return f(*args, **kwargs) + return error_response(message=CommonMsg.user_has_other_locked_nin) return f(*args, **kwargs) From 0a47e435d120277829cd1816286bbc44bd682b89 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Tue, 19 Nov 2024 14:46:27 +0000 Subject: [PATCH 05/12] add tests --- .../common/api/tests/test_nin_helpers.py | 60 +++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/src/eduid/webapp/common/api/tests/test_nin_helpers.py b/src/eduid/webapp/common/api/tests/test_nin_helpers.py index fc5ec984c..df2e5bedd 100644 --- a/src/eduid/webapp/common/api/tests/test_nin_helpers.py +++ b/src/eduid/webapp/common/api/tests/test_nin_helpers.py @@ -64,6 +64,7 @@ def setUp(self, config: SetupConfig | None = None) -> None: super().setUp(config=config) self.test_user_nin = "200001023456" self.wrong_test_user_nin = "199909096789" + self.locked_test_user_nin = "197801011234" self.test_userdata = self.test_user.to_dict() self.test_proofing_user = ProofingUser.from_dict(data=self.test_userdata) @@ -73,12 +74,14 @@ def navet_response(self) -> FullPostalAddress: name=navet_data.person.name, official_address=navet_data.person.postal_addresses.official_address ) - def insert_verified_user(self) -> User: + def insert_verified_user(self, nin: str | None = None) -> User: user = self.app.central_userdb.get_user_by_eppn(self.test_user.eppn) user.identities = IdentityList() + if nin is None: + nin = self.test_user_nin nin_element = NinIdentity.from_dict( dict( - number=self.test_user_nin, + number=nin, created_by="AlreadyVerifiedNinHelpersTest", verified=True, ) @@ -87,12 +90,14 @@ def insert_verified_user(self) -> User: self.app.central_userdb.save(user) return self.app.central_userdb.get_user_by_eppn(user.eppn) - def insert_not_verified_user(self) -> User: + def insert_not_verified_user(self, nin: str | None = None) -> User: user = self.app.central_userdb.get_user_by_eppn(self.test_user.eppn) user.identities = IdentityList() + if nin is None: + nin = self.test_user_nin nin_element = NinIdentity.from_dict( dict( - number=self.test_user_nin, + number=nin, created_by="AlreadyAddedNinHelpersTest", verified=False, ) @@ -309,6 +314,32 @@ def test_verify_wrong_nin_for_user_existing_not_verified(self) -> None: with pytest.raises(LockedIdentityViolation): verify_nin_for_user(user, proofing_state, proofing_log_entry) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_verify_changed_nin_for_user_existing_not_verified(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = self.locked_test_user_nin + user = self.insert_not_verified_user(nin=self.locked_test_user_nin) + nin_element = NinProofingElement.from_dict( + dict(number=self.test_user_nin, created_by="NinHelpersTest", verified=False) + ) + proofing_state = NinProofingState.from_dict({"eduPersonPrincipalName": user.eppn, "nin": nin_element.to_dict()}) + assert proofing_state.nin.created_by is not None + assert nin_element.created_by + proofing_log_entry = self._get_nin_eid_proofing_log_entry( + user=user, created_by=nin_element.created_by, nin=nin_element.number + ) + with self.app.app_context(): + assert verify_nin_for_user(user, proofing_state, proofing_log_entry) is True + user = self.app.private_userdb.get_user_by_eppn(user.eppn) + + self._check_nin_verified_ok( + user=user, proofing_state=proofing_state, number=self.test_user_nin, created_by="NinHelpersTest" + ) + # user should be updated with updated nin as it references old locked nin + assert user.identities.nin is not None + assert user.identities.nin.number == self.test_user_nin + assert user.locked_identity.nin is not None + assert user.locked_identity.nin.number == self.test_user_nin + def test_verify_nin_for_user_existing_verified(self) -> None: user = self.insert_verified_user() nin_element = NinProofingElement.from_dict( @@ -323,6 +354,27 @@ def test_verify_nin_for_user_existing_verified(self) -> None: with self.app.app_context(): assert verify_nin_for_user(user, proofing_state, proofing_log_entry) is True + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_verify_changed_nin_for_user_existing_verified(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = self.locked_test_user_nin + user = self.insert_verified_user(nin=self.locked_test_user_nin) + nin_element = NinProofingElement.from_dict( + dict(number=self.test_user_nin, created_by="NinHelpersTest", verified=False) + ) + proofing_state = NinProofingState.from_dict({"eduPersonPrincipalName": user.eppn, "nin": nin_element.to_dict()}) + assert proofing_state.nin.created_by is not None + assert nin_element.created_by + proofing_log_entry = self._get_nin_eid_proofing_log_entry( + user=user, created_by=nin_element.created_by, nin=nin_element.number + ) + with self.app.app_context(): + assert verify_nin_for_user(user, proofing_state, proofing_log_entry) is True + # The user should not be updated as the old nin is locked and verified + assert user.identities.nin is not None + assert user.identities.nin.number == self.locked_test_user_nin + assert user.locked_identity.nin is not None + assert user.locked_identity.nin.number == self.locked_test_user_nin + def test_verify_nin_with_faulty_proofing_log_element(self) -> None: user = self.insert_no_nins_user() nin_element = NinProofingElement.from_dict( From 13570a9385082342bc992df77d4748b8c515e666 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Tue, 19 Nov 2024 14:47:02 +0000 Subject: [PATCH 06/12] patch other tests --- src/eduid/webapp/bankid/tests/test_app.py | 14 ++++- .../common/api/tests/test_nin_helpers.py | 24 ++++++-- src/eduid/webapp/eidas/tests/test_app.py | 36 ++++++++--- src/eduid/webapp/freja_eid/tests/test_app.py | 4 +- .../webapp/letter_proofing/tests/test_app.py | 24 ++++++-- .../lookup_mobile_proofing/tests/test_app.py | 8 ++- .../webapp/oidc_proofing/tests/test_app.py | 60 ++++++++++++++++--- src/eduid/webapp/svipe_id/tests/test_app.py | 6 +- 8 files changed, 143 insertions(+), 33 deletions(-) diff --git a/src/eduid/webapp/bankid/tests/test_app.py b/src/eduid/webapp/bankid/tests/test_app.py index f59736aca..b021e6458 100644 --- a/src/eduid/webapp/bankid/tests/test_app.py +++ b/src/eduid/webapp/bankid/tests/test_app.py @@ -535,9 +535,13 @@ def test_mfa_token_verify_wrong_verified_nin(self) -> None: self._verify_user_parameters(eppn, identity=nin, identity_present=False) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_mfa_token_verify_no_verified_nin(self, mock_request_user_sync: MagicMock) -> None: + def test_mfa_token_verify_no_verified_nin( + self, mock_request_user_sync: MagicMock, mock_reference_nin: MagicMock + ) -> None: mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn nin = self.test_user_nin @@ -691,9 +695,11 @@ def test_webauthn_token_verify_backdoor(self, mock_request_user_sync: MagicMock) self._verify_user_parameters(eppn, identity=nin, identity_verified=True, token_verified=True, num_proofings=2) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_nin_verify(self, mock_request_user_sync: MagicMock) -> None: + def test_nin_verify(self, mock_request_user_sync: MagicMock, mock_reference_nin: MagicMock) -> None: mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn self._verify_user_parameters(eppn, num_mfa_tokens=0, identity_verified=False) @@ -754,9 +760,11 @@ def test_mfa_login_no_nin(self) -> None: self._verify_user_parameters(eppn, num_mfa_tokens=0, identity_verified=False, num_proofings=0) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_mfa_login_unverified_nin(self, mock_request_user_sync: MagicMock) -> None: + def test_mfa_login_unverified_nin(self, mock_request_user_sync: MagicMock, mock_reference_nin: MagicMock) -> None: mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn # Add locked nin to user diff --git a/src/eduid/webapp/common/api/tests/test_nin_helpers.py b/src/eduid/webapp/common/api/tests/test_nin_helpers.py index df2e5bedd..891f8bcf4 100644 --- a/src/eduid/webapp/common/api/tests/test_nin_helpers.py +++ b/src/eduid/webapp/common/api/tests/test_nin_helpers.py @@ -233,7 +233,9 @@ def test_add_nin_to_user_existing_verified(self) -> None: with pytest.raises(UserDoesNotExist): self.app.private_userdb.get_user_by_eppn(user.eppn) - def test_verify_nin_for_user_navet(self) -> None: + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_verify_nin_for_user_navet(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = self.test_user_nin user = self.insert_no_nins_user() nin_element = NinProofingElement.from_dict( dict(number=self.test_user_nin, created_by="NinHelpersTest", verified=False) @@ -244,7 +246,9 @@ def test_verify_nin_for_user_navet(self) -> None: ) self._test_verify_nin_for_user(user=user, nin_element=nin_element, proofing_log_entry=proofing_log_entry) - def test_verify_nin_for_user_eid(self) -> None: + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_verify_nin_for_user_eid(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = None user = self.insert_no_nins_user() nin_element = NinProofingElement.from_dict( dict(number=self.test_user_nin, created_by="NinHelpersTest", verified=False) @@ -255,7 +259,9 @@ def test_verify_nin_for_user_eid(self) -> None: ) self._test_verify_nin_for_user(user=user, nin_element=nin_element, proofing_log_entry=proofing_log_entry) - def test_verify_nin_for_proofing_user_navet(self) -> None: + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_verify_nin_for_proofing_user_navet(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = None user = self.insert_no_nins_user() nin_element = NinProofingElement.from_dict( dict(number=self.test_user_nin, created_by="NinHelpersTest", verified=False) @@ -268,7 +274,9 @@ def test_verify_nin_for_proofing_user_navet(self) -> None: user=user, nin_element=nin_element, proofing_log_entry=proofing_log_entry ) - def test_verify_nin_for_proofing_user_eid(self) -> None: + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_verify_nin_for_proofing_user_eid(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = None user = self.insert_no_nins_user() nin_element = NinProofingElement.from_dict( dict(number=self.test_user_nin, created_by="NinHelpersTest", verified=False) @@ -281,7 +289,9 @@ def test_verify_nin_for_proofing_user_eid(self) -> None: user=user, nin_element=nin_element, proofing_log_entry=proofing_log_entry ) - def test_verify_nin_for_user_existing_not_verified(self) -> None: + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_verify_nin_for_user_existing_not_verified(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = None user = self.insert_not_verified_user() nin_element = NinProofingElement.from_dict( dict(number=self.test_user_nin, created_by="NinHelpersTest", verified=False) @@ -299,7 +309,9 @@ def test_verify_nin_for_user_existing_not_verified(self) -> None: user=user, proofing_state=proofing_state, number=self.test_user_nin, created_by="AlreadyAddedNinHelpersTest" ) - def test_verify_wrong_nin_for_user_existing_not_verified(self) -> None: + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_verify_wrong_nin_for_user_existing_not_verified(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = None user = self.insert_not_verified_user() nin_element = NinProofingElement.from_dict( dict(number=self.wrong_test_user_nin, created_by="NinHelpersTest", verified=False) diff --git a/src/eduid/webapp/eidas/tests/test_app.py b/src/eduid/webapp/eidas/tests/test_app.py index efac7eda6..b1010cbab 100644 --- a/src/eduid/webapp/eidas/tests/test_app.py +++ b/src/eduid/webapp/eidas/tests/test_app.py @@ -608,13 +608,15 @@ def test_mfa_token_verify_wrong_verified_nin(self) -> None: self._verify_user_parameters(eppn, identity=nin, identity_present=False) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_all_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_mfa_token_verify_no_verified_nin( - self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock + self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock, mock_reference_nin: MagicMock ) -> None: mock_get_all_navet_data.return_value = self._get_all_navet_data() mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn nin = self.test_user_nin @@ -743,9 +745,13 @@ def test_mfa_token_verify_auth_fail(self) -> None: self._verify_user_parameters(eppn) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_webauthn_token_verify_backdoor(self, mock_request_user_sync: MagicMock) -> None: + def test_webauthn_token_verify_backdoor( + self, mock_request_user_sync: MagicMock, mock_reference_nin: MagicMock + ) -> None: mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn nin = self.test_backdoor_nin @@ -768,11 +774,15 @@ def test_webauthn_token_verify_backdoor(self, mock_request_user_sync: MagicMock) self._verify_user_parameters(eppn, identity=nin, identity_verified=True, token_verified=True, num_proofings=2) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_all_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_nin_verify(self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock) -> None: + def test_nin_verify( + self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock, mock_reference_nin: MagicMock + ) -> None: mock_get_all_navet_data.return_value = self._get_all_navet_data() mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn self._verify_user_parameters(eppn, num_mfa_tokens=0, identity_verified=False) @@ -835,13 +845,15 @@ def test_mfa_login_no_nin(self) -> None: self._verify_user_parameters(eppn, num_mfa_tokens=0, identity_verified=False, num_proofings=0) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_all_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_mfa_login_unverified_nin( - self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock + self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock, mock_reference_nin: MagicMock ) -> None: mock_get_all_navet_data.return_value = self._get_all_navet_data() mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn # Add locked nin to user @@ -1100,9 +1112,11 @@ def test_mfa_login_backdoor(self, mock_request_user_sync: MagicMock) -> None: self._verify_user_parameters(eppn, num_mfa_tokens=0, identity_verified=True, num_proofings=0) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_nin_verify_backdoor(self, mock_request_user_sync: MagicMock) -> None: + def test_nin_verify_backdoor(self, mock_request_user_sync: MagicMock, mock_reference_nin: MagicMock) -> None: mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn nin = self.test_backdoor_nin @@ -1122,13 +1136,15 @@ def test_nin_verify_backdoor(self, mock_request_user_sync: MagicMock) -> None: self._verify_user_parameters(eppn, num_mfa_tokens=0, identity=nin, identity_verified=True, num_proofings=1) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_all_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_nin_verify_no_backdoor_in_pro( - self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock + self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock, mock_reference_nin: MagicMock ) -> None: mock_get_all_navet_data.return_value = self._get_all_navet_data() mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn nin = self.test_backdoor_nin @@ -1153,13 +1169,15 @@ def test_nin_verify_no_backdoor_in_pro( eppn, identity=self.test_user_nin, num_mfa_tokens=0, num_proofings=1, identity_verified=True ) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_all_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_nin_verify_no_backdoor_misconfigured( - self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock + self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock, mock_reference_nin: MagicMock ) -> None: mock_get_all_navet_data.return_value = self._get_all_navet_data() mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn nin = self.test_backdoor_nin @@ -1252,13 +1270,15 @@ def test_mfa_authentication_wrong_nin(self) -> None: identity=self.test_user_wrong_nin, ) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_all_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_nin_staging_remap_verify( - self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock + self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock, mock_reference_nin: MagicMock ) -> None: mock_get_all_navet_data.return_value = self._get_all_navet_data() mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn remapped_nin = NinIdentity(number="190102031234") diff --git a/src/eduid/webapp/freja_eid/tests/test_app.py b/src/eduid/webapp/freja_eid/tests/test_app.py index 2bd76e1d9..6b59608a8 100644 --- a/src/eduid/webapp/freja_eid/tests/test_app.py +++ b/src/eduid/webapp/freja_eid/tests/test_app.py @@ -335,9 +335,11 @@ def test_verify_identity_request(self) -> None: ], f"{query['scope']} != {[' '.join(self.app.conf.freja_eid_client.scopes)]}" assert query["code_challenge_method"] == ["S256"] + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_verify_nin_identity(self, mock_request_user_sync: MagicMock) -> None: + def test_verify_nin_identity(self, mock_request_user_sync: MagicMock, mock_reference_nin: MagicMock) -> None: mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.unverified_test_user.eppn country = countries.get("Sweden") diff --git a/src/eduid/webapp/letter_proofing/tests/test_app.py b/src/eduid/webapp/letter_proofing/tests/test_app.py index 38d489710..41461ec3f 100644 --- a/src/eduid/webapp/letter_proofing/tests/test_app.py +++ b/src/eduid/webapp/letter_proofing/tests/test_app.py @@ -243,7 +243,10 @@ def test_letter_sent_status(self) -> None: expires_string = expires.strftime("%Y-%m-%d") self.assertIsInstance(expires_string, str) - def test_verify_letter_code(self) -> None: + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_verify_letter_code(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = None + response1 = self.send_letter(self.test_user_nin) proofing_state = self.app.proofing_statedb.get_state_by_eppn(self.test_user_eppn) # Deliberately test the CSRF token from the send_letter response, @@ -307,7 +310,10 @@ def test_verify_letter_expired(self) -> None: response, type_="POST_LETTER_PROOFING_VERIFY_CODE_FAIL", msg=LetterMsg.letter_expired ) - def test_proofing_flow(self) -> None: + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_proofing_flow(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = None + self.send_letter(self.test_user_nin) self.get_state() proofing_state = self.app.proofing_statedb.get_state_by_eppn(self.test_user_eppn) @@ -319,7 +325,10 @@ def test_proofing_flow(self) -> None: user = self.app.private_userdb.get_user_by_eppn(self.test_user_eppn) self._check_nin_verified_ok(user=user, proofing_state=proofing_state, number=self.test_user_nin) - def test_proofing_flow_previously_added_nin(self) -> None: + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_proofing_flow_previously_added_nin(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = None + user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) not_verified_nin = NinIdentity(number=self.test_user_nin, created_by="test", is_verified=False) user.identities.add(not_verified_nin) @@ -337,7 +346,10 @@ def test_proofing_flow_previously_added_nin(self) -> None: user=user, proofing_state=proofing_state, number=self.test_user_nin, created_by=not_verified_nin.created_by ) - def test_proofing_flow_previously_added_wrong_nin(self) -> None: + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_proofing_flow_previously_added_wrong_nin(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = None + # Send letter to correct nin self.send_letter(self.test_user_nin) @@ -420,13 +432,15 @@ def test_locked_identity_correct_nin( with self.session_cookie(self.browser, self.test_user_eppn): self.send_letter(self.test_user_nin) + @patch("eduid.webapp.common.api.decorators.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_postal_address") def test_locked_identity_incorrect_nin( - self, mock_get_postal_address: MagicMock, mock_request_user_sync: MagicMock + self, mock_get_postal_address: MagicMock, mock_request_user_sync: MagicMock, mock_reference_nin: MagicMock ) -> None: mock_get_postal_address.return_value = self._get_full_postal_address() mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) user.locked_identity.add(NinIdentity(number=self.test_user_nin, created_by="test", is_verified=True)) diff --git a/src/eduid/webapp/lookup_mobile_proofing/tests/test_app.py b/src/eduid/webapp/lookup_mobile_proofing/tests/test_app.py index a4138405c..901392880 100644 --- a/src/eduid/webapp/lookup_mobile_proofing/tests/test_app.py +++ b/src/eduid/webapp/lookup_mobile_proofing/tests/test_app.py @@ -173,15 +173,21 @@ def test_proofing_flow_LookupMobileTaskFailed( user = self.app.private_userdb.get_user_by_eppn(self.test_user_eppn) self._check_nin_not_verified_no_proofing_state(user=user) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.lookup_mobile_relay.LookupMobileRelay.find_nin_by_mobile") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_postal_address") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_proofing_flow_no_match_backdoor( - self, mock_request_user_sync: MagicMock, mock_get_postal_address: MagicMock, mock_find_nin_by_mobile: MagicMock + self, + mock_request_user_sync: MagicMock, + mock_get_postal_address: MagicMock, + mock_find_nin_by_mobile: MagicMock, + mock_reference_nin: MagicMock, ) -> None: mock_find_nin_by_mobile.return_value = None mock_get_postal_address.return_value = None mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None self.app.conf.magic_cookie = "magic-cookie" self.app.conf.magic_cookie_name = "magic-cookie" diff --git a/src/eduid/webapp/oidc_proofing/tests/test_app.py b/src/eduid/webapp/oidc_proofing/tests/test_app.py index 8511a63d1..71c149fe0 100644 --- a/src/eduid/webapp/oidc_proofing/tests/test_app.py +++ b/src/eduid/webapp/oidc_proofing/tests/test_app.py @@ -176,15 +176,21 @@ def test_get_freja_state_bad_csrf(self) -> None: self.assertEqual(response_json["type"], "POST_OIDC_PROOFING_FREJA_PROOFING_FAIL") self.assertEqual(response_json["payload"]["error"]["csrf_token"], ["CSRF failed to validate"]) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.webapp.oidc_proofing.helpers.do_authn_request") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_postal_address") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_seleg_flow( - self, mock_request_user_sync: MagicMock, mock_get_postal_address: MagicMock, mock_oidc_call: MagicMock + self, + mock_request_user_sync: MagicMock, + mock_get_postal_address: MagicMock, + mock_oidc_call: MagicMock, + mock_reference_nin: MagicMock, ) -> None: mock_oidc_call.return_value = True mock_get_postal_address.return_value = self.mock_address mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) with self.session_cookie(self.browser, self.test_user_eppn) as browser: @@ -270,15 +276,21 @@ def test_seleg_flow_low_score( user = self.app.private_userdb.get_user_by_eppn(self.test_user_eppn) self._check_nin_not_verified(user=user, number=self.test_user_nin) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.webapp.oidc_proofing.helpers.do_authn_request") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_postal_address") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_seleg_flow_previously_added_nin( - self, mock_request_user_sync: MagicMock, mock_get_postal_address: MagicMock, mock_oidc_call: MagicMock + self, + mock_request_user_sync: MagicMock, + mock_get_postal_address: MagicMock, + mock_oidc_call: MagicMock, + mock_reference_nin: MagicMock, ) -> None: mock_oidc_call.return_value = True mock_get_postal_address.return_value = self.mock_address mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) not_verified_nin = NinIdentity(number=self.test_user_nin, created_by="test", is_verified=False) @@ -320,15 +332,21 @@ def test_seleg_flow_previously_added_nin( user=user, proofing_state=proofing_state, number=self.test_user_nin, created_by=not_verified_nin.created_by ) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.webapp.oidc_proofing.helpers.do_authn_request") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_postal_address") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_seleg_flow_previously_added_wrong_nin( - self, mock_request_user_sync: MagicMock, mock_get_postal_address: MagicMock, mock_oidc_call: MagicMock + self, + mock_request_user_sync: MagicMock, + mock_get_postal_address: MagicMock, + mock_oidc_call: MagicMock, + mock_reference_nin: MagicMock, ) -> None: mock_oidc_call.return_value = True mock_get_postal_address.return_value = self.mock_address mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) not_verified_nin = NinIdentity(number=self.test_user_wrong_nin, created_by="test", is_verified=False) @@ -368,15 +386,21 @@ def test_seleg_flow_previously_added_wrong_nin( user = self.app.private_userdb.get_user_by_eppn(self.test_user_eppn) self._check_nin_verified_ok(user=user, proofing_state=proofing_state, number=self.test_user_nin) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.webapp.oidc_proofing.helpers.do_authn_request") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_postal_address") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_freja_flow( - self, mock_request_user_sync: MagicMock, mock_get_postal_address: MagicMock, mock_oidc_call: MagicMock + self, + mock_request_user_sync: MagicMock, + mock_get_postal_address: MagicMock, + mock_oidc_call: MagicMock, + mock_reference_nin: MagicMock, ) -> None: mock_oidc_call.return_value = True mock_get_postal_address.return_value = self.mock_address mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None with self.session_cookie(self.browser, self.test_user_eppn) as browser: response = json.loads(browser.get("/freja/proofing").data) @@ -415,15 +439,21 @@ def test_freja_flow( user = self.app.private_userdb.get_user_by_eppn(self.test_user_eppn) self._check_nin_verified_ok(user=user, proofing_state=proofing_state, number=self.test_user_nin) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.webapp.oidc_proofing.helpers.do_authn_request") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_postal_address") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_freja_flow_previously_added_nin( - self, mock_request_user_sync: MagicMock, mock_get_postal_address: MagicMock, mock_oidc_call: MagicMock + self, + mock_request_user_sync: MagicMock, + mock_get_postal_address: MagicMock, + mock_oidc_call: MagicMock, + mock_reference_nin: MagicMock, ) -> None: mock_oidc_call.return_value = True mock_get_postal_address.return_value = self.mock_address mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) not_verified_nin = NinIdentity(number=self.test_user_nin, created_by="test", is_verified=False) @@ -463,15 +493,21 @@ def test_freja_flow_previously_added_nin( user=user, proofing_state=proofing_state, number=self.test_user_nin, created_by=not_verified_nin.created_by ) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.webapp.oidc_proofing.helpers.do_authn_request") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_postal_address") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") def test_freja_flow_previously_added_wrong_nin( - self, mock_request_user_sync: MagicMock, mock_get_postal_address: MagicMock, mock_oidc_call: MagicMock + self, + mock_request_user_sync: MagicMock, + mock_get_postal_address: MagicMock, + mock_oidc_call: MagicMock, + mock_reference_nin: MagicMock, ) -> None: mock_oidc_call.return_value = True mock_get_postal_address.return_value = self.mock_address mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) not_verified_nin = NinIdentity(number=self.test_user_wrong_nin, created_by="test", is_verified=False) @@ -541,11 +577,15 @@ def test_freja_flow_expired_state( # Check that the expired proofing state was removed assert not self.app.proofing_statedb.get_state_by_eppn(self.test_user_eppn) + @patch("eduid.webapp.common.api.decorators.get_reference_nin_from_navet_data") @patch("eduid.webapp.oidc_proofing.helpers.do_authn_request") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_seleg_locked_identity(self, mock_request_user_sync: MagicMock, mock_oidc_call: MagicMock) -> None: + def test_seleg_locked_identity( + self, mock_request_user_sync: MagicMock, mock_oidc_call: MagicMock, mock_reference_nin: MagicMock + ) -> None: mock_oidc_call.return_value = True mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) with self.session_cookie(self.browser, self.test_user_eppn) as browser: @@ -583,11 +623,15 @@ def test_seleg_locked_identity(self, mock_request_user_sync: MagicMock, mock_oid response = json.loads(response.data) self.assertEqual(response["type"], "POST_OIDC_PROOFING_PROOFING_FAIL") + @patch("eduid.webapp.common.api.decorators.get_reference_nin_from_navet_data") @patch("eduid.webapp.oidc_proofing.helpers.do_authn_request") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_freja_locked_identity(self, mock_request_user_sync: MagicMock, mock_oidc_call: MagicMock) -> None: + def test_freja_locked_identity( + self, mock_request_user_sync: MagicMock, mock_oidc_call: MagicMock, mock_reference_nin: MagicMock + ) -> None: mock_oidc_call.return_value = True mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) with self.session_cookie(self.browser, self.test_user_eppn) as browser: diff --git a/src/eduid/webapp/svipe_id/tests/test_app.py b/src/eduid/webapp/svipe_id/tests/test_app.py index 3f18ee9a2..20984d0b5 100644 --- a/src/eduid/webapp/svipe_id/tests/test_app.py +++ b/src/eduid/webapp/svipe_id/tests/test_app.py @@ -309,11 +309,15 @@ def test_verify_identity_request(self) -> None: assert query["acr_values"] == ["face_present"] assert query["claims"] == [json.dumps({"userinfo": self.app.conf.svipe_client.claims_request})] + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.msg_relay.MsgRelay.get_all_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_verify_nin_identity(self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock) -> None: + def test_verify_nin_identity( + self, mock_request_user_sync: MagicMock, mock_get_all_navet_data: MagicMock, mock_reference_nin: MagicMock + ) -> None: mock_get_all_navet_data.return_value = self._get_all_navet_data() mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.unverified_test_user.eppn country = countries.get("Sweden") From 2ef5a9607d01f92d1f2f177ebd20283d6d2992a9 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Tue, 19 Nov 2024 14:47:16 +0000 Subject: [PATCH 07/12] make reformat --- src/eduid/satosa/scimapi/static_attributes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/eduid/satosa/scimapi/static_attributes.py b/src/eduid/satosa/scimapi/static_attributes.py index d5e37032d..3a84d0c62 100644 --- a/src/eduid/satosa/scimapi/static_attributes.py +++ b/src/eduid/satosa/scimapi/static_attributes.py @@ -78,7 +78,6 @@ def _build_static(self, requester: str, vidp: str, existing_attributes: dict) -> else: static_attributes[attr_name] = fmt - logger.debug(f"Appending static attribute {attr_name}: {fmt} for requester {requester} or {vidp}") return static_attributes From e01479bb042199ec7089474ebad0b0fb7a018b20 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Tue, 19 Nov 2024 16:53:46 +0000 Subject: [PATCH 08/12] fix missed tests --- src/eduid/webapp/bankid/tests/test_app.py | 4 +++- src/eduid/webapp/eidas/tests/test_app.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/eduid/webapp/bankid/tests/test_app.py b/src/eduid/webapp/bankid/tests/test_app.py index 537066a73..6e3e00bf2 100644 --- a/src/eduid/webapp/bankid/tests/test_app.py +++ b/src/eduid/webapp/bankid/tests/test_app.py @@ -776,9 +776,11 @@ def test_nin_verify(self, mock_request_user_sync: MagicMock, mock_reference_nin: assert doc["given_name"] == "Ûlla" assert doc["surname"] == "Älm" + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_nin_verify_signup_auth(self, mock_request_user_sync: MagicMock) -> None: + def test_nin_verify_signup_auth(self, mock_request_user_sync: MagicMock, mock_reference_nin: MagicMock) -> None: mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn self._verify_user_parameters(eppn, num_mfa_tokens=0, identity_verified=False) diff --git a/src/eduid/webapp/eidas/tests/test_app.py b/src/eduid/webapp/eidas/tests/test_app.py index 43dc2b736..95cd5a01a 100644 --- a/src/eduid/webapp/eidas/tests/test_app.py +++ b/src/eduid/webapp/eidas/tests/test_app.py @@ -864,9 +864,11 @@ def test_nin_verify( assert doc["given_name"] == "Ûlla" assert doc["surname"] == "Älm" + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") @patch("eduid.common.rpc.am_relay.AmRelay.request_user_sync") - def test_nin_verify_signup_auth(self, mock_request_user_sync: MagicMock) -> None: + def test_nin_verify_signup_auth(self, mock_request_user_sync: MagicMock, mock_reference_nin: MagicMock) -> None: mock_request_user_sync.side_effect = self.request_user_sync + mock_reference_nin.return_value = None eppn = self.test_unverified_user_eppn self._verify_user_parameters(eppn, num_mfa_tokens=0, identity_verified=False) From 6da3213ccabcbbe5e8fa51f5844f132f5fa89545 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Thu, 28 Nov 2024 10:54:00 +0000 Subject: [PATCH 09/12] make am do the lock replacement when applicable --- src/eduid/userdb/proofing/user.py | 2 ++ src/eduid/webapp/common/api/helpers.py | 15 ++++++++------- src/eduid/workers/am/ams/common.py | 22 ++++++++++++++++++++++ src/eduid/workers/am/consistency_checks.py | 22 ++++++++++++++++------ src/eduid/workers/am/tasks.py | 3 ++- 5 files changed, 50 insertions(+), 14 deletions(-) diff --git a/src/eduid/userdb/proofing/user.py b/src/eduid/userdb/proofing/user.py index a8b3615e8..d89c943aa 100644 --- a/src/eduid/userdb/proofing/user.py +++ b/src/eduid/userdb/proofing/user.py @@ -1,7 +1,9 @@ from eduid.userdb import User +from eduid.userdb.identity import IdentityType __author__ = "lundberg" class ProofingUser(User): + replace_lock: IdentityType | None = None pass diff --git a/src/eduid/webapp/common/api/helpers.py b/src/eduid/webapp/common/api/helpers.py index a372771df..42faf1019 100644 --- a/src/eduid/webapp/common/api/helpers.py +++ b/src/eduid/webapp/common/api/helpers.py @@ -239,18 +239,19 @@ def verify_nin_for_user( if reference_nin is not None: current_app.logger.debug(f"verified nin has reference_nin: {reference_nin}") + if ( + proofing_user.locked_identity.nin is not None + and proofing_user.locked_identity.nin.number != proofing_state.nin.number + and proofing_user.locked_identity.nin.number != reference_nin + ): + raise LockedIdentityViolation("users locked nin does not match verified nin or reference nin") + # check if the users current nin is the same as the one just verified # if there is no locked nin identity or the locked nin identity matches we can replace the current nin identity # with the one just verified if proofing_user.identities.nin.number != proofing_state.nin.number: current_app.logger.info("users current nin does not match the nin just verified") current_app.logger.debug(f"{proofing_user.identities.nin.number} != {proofing_state.nin.number}") - if ( - proofing_user.locked_identity.nin is not None - and proofing_user.locked_identity.nin.number != proofing_state.nin.number - and proofing_user.locked_identity.nin.number != reference_nin - ): - raise LockedIdentityViolation("users locked nin does not match verified nin or reference nin") # user has no locked nin identity or the user has previously verified the nin # replace the never verified nin with the one just verified @@ -279,7 +280,7 @@ def verify_nin_for_user( and proofing_user.locked_identity.nin is not None and proofing_user.locked_identity.nin.number == reference_nin ): - proofing_user.locked_identity.replace(element=nin_identity) + proofing_user.replace_lock = IdentityType.NIN # Update users name proofing_user = set_user_names_from_nin_proofing(user=proofing_user, proofing_log_entry=proofing_log_entry) diff --git a/src/eduid/workers/am/ams/common.py b/src/eduid/workers/am/ams/common.py index dd53a6bb0..71a8fe8ff 100644 --- a/src/eduid/workers/am/ams/common.py +++ b/src/eduid/workers/am/ams/common.py @@ -8,6 +8,8 @@ from eduid.common.config.workers import AmConfig from eduid.userdb.exceptions import UserDoesNotExist +from eduid.userdb.identity import IdentityType +from eduid.userdb.proofing.user import ProofingUser from eduid.userdb.user import User from eduid.userdb.userdb import UserDB from eduid.userdb.util import format_dict_for_debug @@ -72,3 +74,23 @@ def fetch_attrs(self, user_id: bson.ObjectId) -> dict[str, Any]: attributes["$unset"] = attributes_unset return attributes + + def get_replace_lock(self, user_id: bson.ObjectId) -> IdentityType | None: + """ + Get the identity type of the lock to be replaced or None if no lock should be replaced. + """ + + logger.debug(f"Trying to get user with _id: {user_id} from {self.private_db}.") + if not self.private_db: + raise RuntimeError("No database initialised") + + user: User | None = self.private_db.get_user_by_id(user_id) + logger.debug(f"User: {user} found.") + if not user: + raise UserDoesNotExist(f"No user found with id {user_id}") + + if not isinstance(user, ProofingUser): + logger.debug("Not a proofing user, returning None.") + return None + + return user.replace_lock diff --git a/src/eduid/workers/am/consistency_checks.py b/src/eduid/workers/am/consistency_checks.py index a2e9f860f..b3ce92ddd 100644 --- a/src/eduid/workers/am/consistency_checks.py +++ b/src/eduid/workers/am/consistency_checks.py @@ -1,3 +1,4 @@ +import copy from typing import Any from bson import ObjectId @@ -5,7 +6,7 @@ from eduid.userdb import LockedIdentityList from eduid.userdb.exceptions import DocumentDoesNotExist, LockedIdentityViolation -from eduid.userdb.identity import IdentityList +from eduid.userdb.identity import IdentityList, IdentityType from eduid.userdb.userdb import AmDB __author__ = "lundberg" @@ -152,7 +153,9 @@ def unverify_identities(userdb: AmDB, user_id: ObjectId, identities: list[dict[s return count -def check_locked_identity(userdb: AmDB, user_id: ObjectId, attributes: dict, app_name: str) -> dict: +def check_locked_identity( + userdb: AmDB, user_id: ObjectId, attributes: dict, app_name: str, replace_lock: IdentityType | None = None +) -> dict: """ :param userdb: Central userdb :param user_id: User document _id @@ -184,17 +187,24 @@ def check_locked_identity(userdb: AmDB, user_id: ObjectId, attributes: dict, app updated = True continue + if replace_lock is locked_identity.identity_type: + # replace the locked identity with the new verified identity + if identity.created_by is None: + identity.created_by = app_name + locked_identities.replace(identity) + updated = True + continue + # there is already an identity of the verified identity type in locked identities # bail if they do not match if identity.unique_value != locked_identity.unique_value: - # XXX: non-persistent identities should be handled in the app verifying the identity - # XXX: by using locked_identities.replace() logger.error(f"Verified identity does not match locked identity for user with id {user_id}") logger.debug(f"identity: {identity}") logger.debug(f"locked_identity: {locked_identity}") raise LockedIdentityViolation(f"Verified nin does not match locked identity for user with id {user_id}") + new_attributes = copy.deepcopy(attributes) if updated: - attributes["$set"]["locked_identity"] = locked_identities.to_list_of_dicts() + new_attributes["$set"]["locked_identity"] = locked_identities.to_list_of_dicts() - return attributes + return new_attributes diff --git a/src/eduid/workers/am/tasks.py b/src/eduid/workers/am/tasks.py index d50df3ab0..8b635a843 100644 --- a/src/eduid/workers/am/tasks.py +++ b/src/eduid/workers/am/tasks.py @@ -71,6 +71,7 @@ def update_attributes_keep_result(self: AttributeManager, app_name: str, user_id try: attributes = attribute_fetcher.fetch_attrs(_id) + replace_lock = attribute_fetcher.get_replace_lock(_id) except UserDoesNotExist as e: logger.error(f"The user {_id} does not exist in the database for plugin {app_name}: {e}") raise e @@ -80,7 +81,7 @@ def update_attributes_keep_result(self: AttributeManager, app_name: str, user_id try: logger.debug(f"Checking locked identity during sync attempt from {app_name}") - attributes = check_locked_identity(self.userdb, _id, attributes, app_name) + attributes = check_locked_identity(self.userdb, _id, attributes, app_name, replace_lock) except LockedIdentityViolation as e: logger.error(e) raise e From f4d8c359fad62566b4814389c302746fbdd09ab8 Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Thu, 28 Nov 2024 10:55:40 +0000 Subject: [PATCH 10/12] use replace_lock signalling to am for foreign ids --- src/eduid/webapp/eidas/proofing.py | 2 +- src/eduid/webapp/freja_eid/proofing.py | 2 +- src/eduid/webapp/svipe_id/proofing.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/eduid/webapp/eidas/proofing.py b/src/eduid/webapp/eidas/proofing.py index 761dad667..4e899a85e 100644 --- a/src/eduid/webapp/eidas/proofing.py +++ b/src/eduid/webapp/eidas/proofing.py @@ -274,7 +274,7 @@ def verify_identity(self, user: User) -> VerifyUserResult: return VerifyUserResult(error=CommonMsg.locked_identity_not_matching) # replace the locked identity as the users asserted prid has changed, # and we are sure enough that it is the same person - proofing_user.locked_identity.replace(element=new_identity) + proofing_user.replace_lock = new_identity.identity_type # the existing identity is not verified, just remove it if existing_identity is not None: diff --git a/src/eduid/webapp/freja_eid/proofing.py b/src/eduid/webapp/freja_eid/proofing.py index 9402ad679..c72eb0d39 100644 --- a/src/eduid/webapp/freja_eid/proofing.py +++ b/src/eduid/webapp/freja_eid/proofing.py @@ -124,7 +124,7 @@ def _verify_foreign_identity(self, user: User) -> VerifyUserResult: return VerifyUserResult(error=CommonMsg.locked_identity_not_matching) # replace the locked identity as the users asserted prid has changed, # and we are sure enough that it is the same person - proofing_user.locked_identity.replace(element=new_identity) + proofing_user.replace_lock = new_identity.identity_type # the existing identity is not verified, just remove it if existing_identity is not None: diff --git a/src/eduid/webapp/svipe_id/proofing.py b/src/eduid/webapp/svipe_id/proofing.py index 24f102ba7..976d88af8 100644 --- a/src/eduid/webapp/svipe_id/proofing.py +++ b/src/eduid/webapp/svipe_id/proofing.py @@ -113,7 +113,7 @@ def _verify_foreign_identity(self, user: User) -> VerifyUserResult: return VerifyUserResult(error=CommonMsg.locked_identity_not_matching) # replace the locked identity as the users asserted prid has changed, # and we are sure enough that it is the same person - proofing_user.locked_identity.replace(element=new_identity) + proofing_user.replace_lock = new_identity.identity_type # the existing identity is not verified, just remove it if existing_identity is not None: From 2bb01ed4423c6ccdc8db74c0b23f3ad6692f1c1b Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Thu, 28 Nov 2024 10:56:22 +0000 Subject: [PATCH 11/12] update tests --- src/eduid/webapp/common/api/testing.py | 11 ++++ .../common/api/tests/test_nin_helpers.py | 50 +++++++++++++++++-- .../webapp/letter_proofing/tests/test_app.py | 41 +++++++++++++++ src/eduid/workers/am/tests/test_tasks.py | 45 ++++++++++++++--- 4 files changed, 136 insertions(+), 11 deletions(-) diff --git a/src/eduid/webapp/common/api/testing.py b/src/eduid/webapp/common/api/testing.py index 17670bcca..eae676690 100644 --- a/src/eduid/webapp/common/api/testing.py +++ b/src/eduid/webapp/common/api/testing.py @@ -24,6 +24,7 @@ from eduid.userdb.db import BaseDB from eduid.userdb.element import ElementKey from eduid.userdb.fixtures.users import UserFixtures +from eduid.userdb.identity import IdentityType from eduid.userdb.logs.db import ProofingLog from eduid.userdb.proofing.state import NinProofingState from eduid.userdb.testing import MongoTemporaryInstance, SetupConfig @@ -271,10 +272,15 @@ def request_user_sync(self, private_user: User, app_name_override: str | None = central_user = self.app.central_userdb.get_user_by_id(private_user.user_id) private_user_dict = private_user.to_dict() + replace_lock: IdentityType | None = None # fix signup_user data if "proofing_reference" in private_user_dict: del private_user_dict["proofing_reference"] + if "replace_lock" in private_user_dict: + replace_lock = private_user_dict["replace_lock"] + del private_user_dict["replace_lock"] + if central_user is None: # This is a new user, create a new user in the central db self.app.central_userdb.save(User.from_dict(private_user_dict)) @@ -303,6 +309,11 @@ def request_user_sync(self, private_user: User, app_name_override: str | None = identity.created_by = "test" user.locked_identity.add(identity) continue + if replace_lock is locked_identity.identity_type: + # replace the locked identity with the new verified identity + if identity.created_by is None: + identity.created_by = "test" + user.locked_identity.replace(identity) # Restore metadata that is necessary for the consistency checks in the save() function user.modified_ts = central_user.modified_ts diff --git a/src/eduid/webapp/common/api/tests/test_nin_helpers.py b/src/eduid/webapp/common/api/tests/test_nin_helpers.py index 891f8bcf4..235f79fe8 100644 --- a/src/eduid/webapp/common/api/tests/test_nin_helpers.py +++ b/src/eduid/webapp/common/api/tests/test_nin_helpers.py @@ -12,7 +12,8 @@ from eduid.userdb import NinIdentity, User from eduid.userdb.exceptions import LockedIdentityViolation, UserDoesNotExist from eduid.userdb.fixtures.users import UserFixtures -from eduid.userdb.identity import IdentityList +from eduid.userdb.identity import IdentityList, IdentityType +from eduid.userdb.locked_identity import LockedIdentityList from eduid.userdb.logs import ProofingLog from eduid.userdb.logs.element import ( ForeignIdProofingLogElement, @@ -106,10 +107,28 @@ def insert_not_verified_user(self, nin: str | None = None) -> User: self.app.central_userdb.save(user) return self.app.central_userdb.get_user_by_eppn(user.eppn) + def insert_not_verified_not_locked_user(self, nin: str | None = None) -> User: + user = self.app.central_userdb.get_user_by_eppn(self.test_user.eppn) + user.identities = IdentityList() + if nin is None: + nin = self.test_user_nin + nin_element = NinIdentity.from_dict( + dict( + number=nin, + created_by="AlreadyAddedNinHelpersTest", + verified=False, + ) + ) + user.identities.add(nin_element) + user.locked_identity = LockedIdentityList() + self.app.central_userdb.save(user) + return self.app.central_userdb.get_user_by_eppn(user.eppn) + def insert_no_nins_user(self) -> User: # Replace user with one without previous proofings user = self.app.central_userdb.get_user_by_eppn(self.test_user.eppn) user.identities = IdentityList() + user.locked_identity = LockedIdentityList() self.app.central_userdb.save(user) return self.app.central_userdb.get_user_by_eppn(user.eppn) @@ -292,7 +311,7 @@ def test_verify_nin_for_proofing_user_eid(self, mock_reference_nin: MagicMock) - @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") def test_verify_nin_for_user_existing_not_verified(self, mock_reference_nin: MagicMock) -> None: mock_reference_nin.return_value = None - user = self.insert_not_verified_user() + user = self.insert_not_verified_not_locked_user() nin_element = NinProofingElement.from_dict( dict(number=self.test_user_nin, created_by="NinHelpersTest", verified=False) ) @@ -309,6 +328,29 @@ def test_verify_nin_for_user_existing_not_verified(self, mock_reference_nin: Mag user=user, proofing_state=proofing_state, number=self.test_user_nin, created_by="AlreadyAddedNinHelpersTest" ) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_verify_nin_for_user_existing_locked_not_verified(self, mock_reference_nin: MagicMock) -> None: + mock_reference_nin.return_value = None + user = self.insert_not_verified_user() + nin_element = NinProofingElement.from_dict( + dict(number=self.locked_test_user_nin, created_by="NinHelpersTest", verified=False) + ) + proofing_state = NinProofingState.from_dict({"eduPersonPrincipalName": user.eppn, "nin": nin_element.to_dict()}) + assert nin_element.created_by is not None + proofing_log_entry = self._get_nin_eid_proofing_log_entry( + user=user, created_by=nin_element.created_by, nin=nin_element.number + ) + with self.app.app_context(): + assert verify_nin_for_user(user, proofing_state, proofing_log_entry) is True + user = self.app.private_userdb.get_user_by_eppn(user.eppn) + + self._check_nin_verified_ok( + user=user, + proofing_state=proofing_state, + number=self.locked_test_user_nin, + created_by="NinHelpersTest", + ) + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") def test_verify_wrong_nin_for_user_existing_not_verified(self, mock_reference_nin: MagicMock) -> None: mock_reference_nin.return_value = None @@ -349,8 +391,8 @@ def test_verify_changed_nin_for_user_existing_not_verified(self, mock_reference_ # user should be updated with updated nin as it references old locked nin assert user.identities.nin is not None assert user.identities.nin.number == self.test_user_nin - assert user.locked_identity.nin is not None - assert user.locked_identity.nin.number == self.test_user_nin + # NIN should be updated by am when saving to main DB + assert user.replace_lock is IdentityType.NIN def test_verify_nin_for_user_existing_verified(self) -> None: user = self.insert_verified_user() diff --git a/src/eduid/webapp/letter_proofing/tests/test_app.py b/src/eduid/webapp/letter_proofing/tests/test_app.py index 41461ec3f..7cb48868a 100644 --- a/src/eduid/webapp/letter_proofing/tests/test_app.py +++ b/src/eduid/webapp/letter_proofing/tests/test_app.py @@ -24,6 +24,7 @@ class LetterProofingTests(EduidAPITestCase[LetterProofingApp]): def setUp(self, config: SetupConfig | None = None) -> None: self.test_user_eppn = "hubba-baar" self.test_user_nin = "200001023456" + self.test_old_user_nin = "199909096789" self.test_user_wrong_nin = "190001021234" if config is None: config = SetupConfig() @@ -371,6 +372,46 @@ def test_proofing_flow_previously_added_wrong_nin(self, mock_reference_nin: Magi user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) self._check_nin_verified_ok(user=user, proofing_state=proofing_state, number=self.test_user_nin) + @patch("eduid.webapp.common.api.decorators.get_reference_nin_from_navet_data") + @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data") + def test_proofing_flow_with_replacement_nin( + self, mock_reference_nin: MagicMock, mock_decorator_nin: MagicMock + ) -> None: + mock_reference_nin.return_value = self.test_old_user_nin + mock_decorator_nin.return_value = self.test_old_user_nin + + # Send letter to old nin and verify + self.send_letter(self.test_old_user_nin) + proofing_state = self.app.proofing_statedb.get_state_by_eppn(self.test_user_eppn) + assert proofing_state is not None + assert proofing_state.nin is not None + assert proofing_state.nin.verification_code is not None + self.verify_code(proofing_state.nin.verification_code, None) + + # Check that old nin is locked in + user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) + assert user.locked_identity.nin is not None + assert user.locked_identity.nin.number == self.test_old_user_nin + + # Replace old nin with new nin + user.identities.remove(ElementKey(IdentityType.NIN)) + not_verified_nin = NinIdentity(number=self.test_user_nin, created_by="test", is_verified=False) + user.identities.add(not_verified_nin) + self.app.central_userdb.save(user) + + # Send letter to new nin and verify + self.send_letter(self.test_user_nin) + new_proofing_state = self.app.proofing_statedb.get_state_by_eppn(self.test_user_eppn) + assert new_proofing_state is not None + assert new_proofing_state.nin is not None + assert new_proofing_state.nin.verification_code is not None + self.verify_code(new_proofing_state.nin.verification_code, None) + + # Check that new nin is locked in + updated_user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) + assert updated_user.locked_identity.nin is not None + assert updated_user.locked_identity.nin.number == self.test_user_nin + def test_expire_proofing_state(self) -> None: self.send_letter(self.test_user_nin) json_data = self.get_state() diff --git a/src/eduid/workers/am/tests/test_tasks.py b/src/eduid/workers/am/tests/test_tasks.py index b889caa38..0ade42c64 100644 --- a/src/eduid/workers/am/tests/test_tasks.py +++ b/src/eduid/workers/am/tests/test_tasks.py @@ -1,6 +1,7 @@ from bson import ObjectId import eduid.userdb +from eduid.common.testing_base import normalised_data from eduid.userdb import LockedIdentityList, NinIdentity from eduid.userdb.exceptions import EduIDUserDBError, MultipleUsersReturned from eduid.userdb.fixtures.users import UserFixtures @@ -193,14 +194,22 @@ def test_unverify_duplicate_multiple_attribute_values(self) -> None: def test_create_locked_identity(self) -> None: user_id = ObjectId("901234567890123456789012") # johnsmith@example.org / babba-labba - attributes = {"$set": {"nins": [{"verified": True, "number": "200102031234", "primary": True}]}} + attributes = { + "$set": { + "identities": [{"identity_type": IdentityType.NIN.value, "number": "200102031234", "verified": True}] + } + } new_attributes = check_locked_identity(self.amdb, user_id, attributes, "test") + # check_locked_identity should create a locked_identity so add it to the expected attributes locked_nin = NinIdentity(number="200102031234", created_by="test", is_verified=True) locked_identities = LockedIdentityList(elements=[locked_nin]) attributes["$set"]["locked_identity"] = locked_identities.to_list_of_dicts() - self.assertDictEqual(attributes, new_attributes) + self.assertDictEqual( + normalised_data(attributes, exclude_keys=["created_ts", "modified_ts"]), + normalised_data(new_attributes, exclude_keys=["created_ts", "modified_ts"]), + ) def test_check_locked_identity(self) -> None: user_id = ObjectId("012345678901234567890123") # johnsmith@example.com / hubba-bubba @@ -212,14 +221,13 @@ def test_check_locked_identity(self) -> None: self.amdb.save(user) attributes = { "$set": { - "nins": [{"verified": True, "number": locked_nin.number, "primary": True}], # hubba-bubba's primary nin + "identities": [ + {"identity_type": IdentityType.NIN.value, "number": locked_nin.number, "verified": True} + ], # hubba-bubba } } new_attributes = check_locked_identity(self.amdb, user_id, attributes, "test") - - locked_identities = LockedIdentityList(elements=[locked_nin]) - attributes["$set"]["locked_identity"] = locked_identities.to_list_of_dicts() - + # user has locked_identity that is the same as the verified identity so only identities should be set self.assertDictEqual(attributes, new_attributes) def test_check_locked_identity_wrong_nin(self) -> None: @@ -236,6 +244,29 @@ def test_check_locked_identity_wrong_nin(self) -> None: with self.assertRaises(EduIDUserDBError): check_locked_identity(self.amdb, user_id, attributes, "test") + def test_check_locked_identity_replace_lock(self) -> None: + user_id = ObjectId("901234567890123456789012") # johnsmith@example.org / babba-labba + user = self.amdb.get_user_by_id(user_id) + assert user + user.locked_identity.add(NinIdentity(number="200102031234", created_by="test", is_verified=True)) + self.amdb.save(user) + attributes = { + "$set": { + "identities": [{"identity_type": IdentityType.NIN.value, "verified": True, "number": "200506076789"}] + } + } + new_attributes = check_locked_identity(self.amdb, user_id, attributes, "test", replace_lock=IdentityType.NIN) + + # check_locked_identity should replace locked identity with new identity + locked_nin = NinIdentity(number="200506076789", created_by="test", is_verified=True) + locked_identities = LockedIdentityList(elements=[locked_nin]) + attributes["$set"]["locked_identity"] = locked_identities.to_list_of_dicts() + + self.assertDictEqual( + normalised_data(attributes, exclude_keys=["created_ts", "modified_ts"]), + normalised_data(new_attributes, exclude_keys=["created_ts", "modified_ts"]), + ) + def test_check_locked_identity_no_verified_nin(self) -> None: user_id = ObjectId("012345678901234567890123") # johnsmith@example.com / hubba-bubba attributes = {"$set": {"phone": [{"verified": True, "number": "+34609609609", "primary": True}]}} From 89fea9e9caf10f4115539d4ad391b039f361088f Mon Sep 17 00:00:00 2001 From: Lasse Yledahl Date: Thu, 28 Nov 2024 14:13:41 +0000 Subject: [PATCH 12/12] rename replace_lock to replace_locked --- src/eduid/userdb/proofing/user.py | 2 +- src/eduid/webapp/common/api/helpers.py | 2 +- src/eduid/webapp/common/api/testing.py | 10 +++++----- src/eduid/webapp/common/api/tests/test_nin_helpers.py | 2 +- src/eduid/webapp/eidas/proofing.py | 2 +- src/eduid/webapp/freja_eid/proofing.py | 2 +- src/eduid/webapp/svipe_id/proofing.py | 2 +- src/eduid/workers/am/ams/common.py | 4 ++-- src/eduid/workers/am/consistency_checks.py | 4 ++-- src/eduid/workers/am/tasks.py | 4 ++-- src/eduid/workers/am/tests/test_tasks.py | 4 ++-- 11 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/eduid/userdb/proofing/user.py b/src/eduid/userdb/proofing/user.py index d89c943aa..d53648578 100644 --- a/src/eduid/userdb/proofing/user.py +++ b/src/eduid/userdb/proofing/user.py @@ -5,5 +5,5 @@ class ProofingUser(User): - replace_lock: IdentityType | None = None + replace_locked: IdentityType | None = None pass diff --git a/src/eduid/webapp/common/api/helpers.py b/src/eduid/webapp/common/api/helpers.py index 42faf1019..999983f97 100644 --- a/src/eduid/webapp/common/api/helpers.py +++ b/src/eduid/webapp/common/api/helpers.py @@ -280,7 +280,7 @@ def verify_nin_for_user( and proofing_user.locked_identity.nin is not None and proofing_user.locked_identity.nin.number == reference_nin ): - proofing_user.replace_lock = IdentityType.NIN + proofing_user.replace_locked = IdentityType.NIN # Update users name proofing_user = set_user_names_from_nin_proofing(user=proofing_user, proofing_log_entry=proofing_log_entry) diff --git a/src/eduid/webapp/common/api/testing.py b/src/eduid/webapp/common/api/testing.py index eae676690..40a93a7c8 100644 --- a/src/eduid/webapp/common/api/testing.py +++ b/src/eduid/webapp/common/api/testing.py @@ -272,14 +272,14 @@ def request_user_sync(self, private_user: User, app_name_override: str | None = central_user = self.app.central_userdb.get_user_by_id(private_user.user_id) private_user_dict = private_user.to_dict() - replace_lock: IdentityType | None = None + replace_locked: IdentityType | None = None # fix signup_user data if "proofing_reference" in private_user_dict: del private_user_dict["proofing_reference"] - if "replace_lock" in private_user_dict: - replace_lock = private_user_dict["replace_lock"] - del private_user_dict["replace_lock"] + if "replace_locked" in private_user_dict: + replace_locked = private_user_dict["replace_locked"] + del private_user_dict["replace_locked"] if central_user is None: # This is a new user, create a new user in the central db @@ -309,7 +309,7 @@ def request_user_sync(self, private_user: User, app_name_override: str | None = identity.created_by = "test" user.locked_identity.add(identity) continue - if replace_lock is locked_identity.identity_type: + if replace_locked is locked_identity.identity_type: # replace the locked identity with the new verified identity if identity.created_by is None: identity.created_by = "test" diff --git a/src/eduid/webapp/common/api/tests/test_nin_helpers.py b/src/eduid/webapp/common/api/tests/test_nin_helpers.py index 235f79fe8..809d436f7 100644 --- a/src/eduid/webapp/common/api/tests/test_nin_helpers.py +++ b/src/eduid/webapp/common/api/tests/test_nin_helpers.py @@ -392,7 +392,7 @@ def test_verify_changed_nin_for_user_existing_not_verified(self, mock_reference_ assert user.identities.nin is not None assert user.identities.nin.number == self.test_user_nin # NIN should be updated by am when saving to main DB - assert user.replace_lock is IdentityType.NIN + assert user.replace_locked is IdentityType.NIN def test_verify_nin_for_user_existing_verified(self) -> None: user = self.insert_verified_user() diff --git a/src/eduid/webapp/eidas/proofing.py b/src/eduid/webapp/eidas/proofing.py index 4e899a85e..9a0e3e598 100644 --- a/src/eduid/webapp/eidas/proofing.py +++ b/src/eduid/webapp/eidas/proofing.py @@ -274,7 +274,7 @@ def verify_identity(self, user: User) -> VerifyUserResult: return VerifyUserResult(error=CommonMsg.locked_identity_not_matching) # replace the locked identity as the users asserted prid has changed, # and we are sure enough that it is the same person - proofing_user.replace_lock = new_identity.identity_type + proofing_user.replace_locked = new_identity.identity_type # the existing identity is not verified, just remove it if existing_identity is not None: diff --git a/src/eduid/webapp/freja_eid/proofing.py b/src/eduid/webapp/freja_eid/proofing.py index c72eb0d39..f06ec68fe 100644 --- a/src/eduid/webapp/freja_eid/proofing.py +++ b/src/eduid/webapp/freja_eid/proofing.py @@ -124,7 +124,7 @@ def _verify_foreign_identity(self, user: User) -> VerifyUserResult: return VerifyUserResult(error=CommonMsg.locked_identity_not_matching) # replace the locked identity as the users asserted prid has changed, # and we are sure enough that it is the same person - proofing_user.replace_lock = new_identity.identity_type + proofing_user.replace_locked = new_identity.identity_type # the existing identity is not verified, just remove it if existing_identity is not None: diff --git a/src/eduid/webapp/svipe_id/proofing.py b/src/eduid/webapp/svipe_id/proofing.py index 976d88af8..53e956a54 100644 --- a/src/eduid/webapp/svipe_id/proofing.py +++ b/src/eduid/webapp/svipe_id/proofing.py @@ -113,7 +113,7 @@ def _verify_foreign_identity(self, user: User) -> VerifyUserResult: return VerifyUserResult(error=CommonMsg.locked_identity_not_matching) # replace the locked identity as the users asserted prid has changed, # and we are sure enough that it is the same person - proofing_user.replace_lock = new_identity.identity_type + proofing_user.replace_locked = new_identity.identity_type # the existing identity is not verified, just remove it if existing_identity is not None: diff --git a/src/eduid/workers/am/ams/common.py b/src/eduid/workers/am/ams/common.py index 71a8fe8ff..bcb3d50e1 100644 --- a/src/eduid/workers/am/ams/common.py +++ b/src/eduid/workers/am/ams/common.py @@ -75,7 +75,7 @@ def fetch_attrs(self, user_id: bson.ObjectId) -> dict[str, Any]: return attributes - def get_replace_lock(self, user_id: bson.ObjectId) -> IdentityType | None: + def get_replace_locked(self, user_id: bson.ObjectId) -> IdentityType | None: """ Get the identity type of the lock to be replaced or None if no lock should be replaced. """ @@ -93,4 +93,4 @@ def get_replace_lock(self, user_id: bson.ObjectId) -> IdentityType | None: logger.debug("Not a proofing user, returning None.") return None - return user.replace_lock + return user.replace_locked diff --git a/src/eduid/workers/am/consistency_checks.py b/src/eduid/workers/am/consistency_checks.py index b3ce92ddd..7c04ebb27 100644 --- a/src/eduid/workers/am/consistency_checks.py +++ b/src/eduid/workers/am/consistency_checks.py @@ -154,7 +154,7 @@ def unverify_identities(userdb: AmDB, user_id: ObjectId, identities: list[dict[s def check_locked_identity( - userdb: AmDB, user_id: ObjectId, attributes: dict, app_name: str, replace_lock: IdentityType | None = None + userdb: AmDB, user_id: ObjectId, attributes: dict, app_name: str, replace_locked: IdentityType | None = None ) -> dict: """ :param userdb: Central userdb @@ -187,7 +187,7 @@ def check_locked_identity( updated = True continue - if replace_lock is locked_identity.identity_type: + if replace_locked is locked_identity.identity_type: # replace the locked identity with the new verified identity if identity.created_by is None: identity.created_by = app_name diff --git a/src/eduid/workers/am/tasks.py b/src/eduid/workers/am/tasks.py index 8b635a843..579940643 100644 --- a/src/eduid/workers/am/tasks.py +++ b/src/eduid/workers/am/tasks.py @@ -71,7 +71,7 @@ def update_attributes_keep_result(self: AttributeManager, app_name: str, user_id try: attributes = attribute_fetcher.fetch_attrs(_id) - replace_lock = attribute_fetcher.get_replace_lock(_id) + replace_locked = attribute_fetcher.get_replace_locked(_id) except UserDoesNotExist as e: logger.error(f"The user {_id} does not exist in the database for plugin {app_name}: {e}") raise e @@ -81,7 +81,7 @@ def update_attributes_keep_result(self: AttributeManager, app_name: str, user_id try: logger.debug(f"Checking locked identity during sync attempt from {app_name}") - attributes = check_locked_identity(self.userdb, _id, attributes, app_name, replace_lock) + attributes = check_locked_identity(self.userdb, _id, attributes, app_name, replace_locked) except LockedIdentityViolation as e: logger.error(e) raise e diff --git a/src/eduid/workers/am/tests/test_tasks.py b/src/eduid/workers/am/tests/test_tasks.py index 0ade42c64..077f2beb3 100644 --- a/src/eduid/workers/am/tests/test_tasks.py +++ b/src/eduid/workers/am/tests/test_tasks.py @@ -244,7 +244,7 @@ def test_check_locked_identity_wrong_nin(self) -> None: with self.assertRaises(EduIDUserDBError): check_locked_identity(self.amdb, user_id, attributes, "test") - def test_check_locked_identity_replace_lock(self) -> None: + def test_check_locked_identity_replace_locked(self) -> None: user_id = ObjectId("901234567890123456789012") # johnsmith@example.org / babba-labba user = self.amdb.get_user_by_id(user_id) assert user @@ -255,7 +255,7 @@ def test_check_locked_identity_replace_lock(self) -> None: "identities": [{"identity_type": IdentityType.NIN.value, "verified": True, "number": "200506076789"}] } } - new_attributes = check_locked_identity(self.amdb, user_id, attributes, "test", replace_lock=IdentityType.NIN) + new_attributes = check_locked_identity(self.amdb, user_id, attributes, "test", replace_locked=IdentityType.NIN) # check_locked_identity should replace locked identity with new identity locked_nin = NinIdentity(number="200506076789", created_by="test", is_verified=True)