diff --git a/src/eduid/userdb/proofing/user.py b/src/eduid/userdb/proofing/user.py index a8b3615e8..d53648578 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_locked: IdentityType | None = None pass diff --git a/src/eduid/webapp/bankid/tests/test_app.py b/src/eduid/webapp/bankid/tests/test_app.py index c7259856e..6e3e00bf2 100644 --- a/src/eduid/webapp/bankid/tests/test_app.py +++ b/src/eduid/webapp/bankid/tests/test_app.py @@ -583,9 +583,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 @@ -739,9 +743,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) @@ -770,9 +776,11 @@ def test_nin_verify(self, mock_request_user_sync: MagicMock) -> None: 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) @@ -836,9 +844,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/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) diff --git a/src/eduid/webapp/common/api/helpers.py b/src/eduid/webapp/common/api/helpers.py index bc89963a6..999983f97 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,17 +235,23 @@ 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}") + + 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 - ): - raise LockedIdentityViolation("users locked nin does not match verified 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 +274,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.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 17670bcca..40a93a7c8 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_locked: IdentityType | None = None # fix signup_user data if "proofing_reference" in private_user_dict: del private_user_dict["proofing_reference"] + 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 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_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" + 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 fc5ec984c..809d436f7 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, @@ -64,6 +65,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 +75,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 +91,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, ) @@ -101,10 +107,28 @@ def insert_not_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_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) @@ -228,7 +252,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) @@ -239,7 +265,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) @@ -250,7 +278,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) @@ -263,7 +293,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) @@ -276,8 +308,10 @@ 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: - user = self.insert_not_verified_user() + @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_not_locked_user() nin_element = NinProofingElement.from_dict( dict(number=self.test_user_nin, created_by="NinHelpersTest", verified=False) ) @@ -294,7 +328,32 @@ 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_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 user = self.insert_not_verified_user() nin_element = NinProofingElement.from_dict( dict(number=self.wrong_test_user_nin, created_by="NinHelpersTest", verified=False) @@ -309,6 +368,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 + # NIN should be updated by am when saving to main DB + assert user.replace_locked is IdentityType.NIN + def test_verify_nin_for_user_existing_verified(self) -> None: user = self.insert_verified_user() nin_element = NinProofingElement.from_dict( @@ -323,6 +408,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( 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 diff --git a/src/eduid/webapp/eidas/proofing.py b/src/eduid/webapp/eidas/proofing.py index 761dad667..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.locked_identity.replace(element=new_identity) + 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/eidas/tests/test_app.py b/src/eduid/webapp/eidas/tests/test_app.py index e3c6b4e15..95cd5a01a 100644 --- a/src/eduid/webapp/eidas/tests/test_app.py +++ b/src/eduid/webapp/eidas/tests/test_app.py @@ -661,13 +661,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 @@ -796,9 +798,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 @@ -821,11 +827,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) @@ -854,9 +864,11 @@ def test_nin_verify(self, mock_request_user_sync: MagicMock, mock_get_all_navet_ 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) @@ -922,13 +934,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 @@ -1187,9 +1201,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 @@ -1209,13 +1225,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 @@ -1240,13 +1258,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 @@ -1339,13 +1359,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/proofing.py b/src/eduid/webapp/freja_eid/proofing.py index 9402ad679..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.locked_identity.replace(element=new_identity) + 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/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..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() @@ -243,7 +244,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 +311,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 +326,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 +347,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) @@ -359,6 +372,46 @@ def test_proofing_flow_previously_added_wrong_nin(self) -> None: 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() @@ -420,13 +473,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/proofing.py b/src/eduid/webapp/svipe_id/proofing.py index 24f102ba7..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.locked_identity.replace(element=new_identity) + 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/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") diff --git a/src/eduid/workers/am/ams/common.py b/src/eduid/workers/am/ams/common.py index dd53a6bb0..bcb3d50e1 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_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. + """ + + 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_locked diff --git a/src/eduid/workers/am/consistency_checks.py b/src/eduid/workers/am/consistency_checks.py index a2e9f860f..7c04ebb27 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_locked: 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_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 + 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..579940643 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_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 @@ -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_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 b889caa38..077f2beb3 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_locked(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_locked=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}]}} 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)