From 6e0e1b2b229219561e93bd6663c366925ca68684 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Tue, 25 Jun 2024 15:29:48 +0200 Subject: [PATCH 01/11] check user settings if mfa should be forced for user --- src/eduid/webapp/idp/mfa_action.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/eduid/webapp/idp/mfa_action.py b/src/eduid/webapp/idp/mfa_action.py index 1d9865fde..9fb821837 100644 --- a/src/eduid/webapp/idp/mfa_action.py +++ b/src/eduid/webapp/idp/mfa_action.py @@ -19,6 +19,10 @@ def need_security_key(user: IdPUser, ticket: LoginContext) -> bool: logger.debug("User has no FIDO credentials, no extra requirement for MFA this session imposed") return False + if user.settings.force_mfa is False: + logger.debug("User has not forced MFA, no extra requirement for MFA this session imposed") + return False + for cred_key in ticket.pending_request.credentials_used: credential: Optional[Credential] if cred_key in ticket.pending_request.onetime_credentials: From 74e0396fc3342ee880a456d4f131353599f28d79 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Tue, 25 Jun 2024 15:32:11 +0200 Subject: [PATCH 02/11] add support for mfa in idp tests --- src/eduid/webapp/idp/tests/test_api.py | 140 +++++++++++++++++++++++-- 1 file changed, 130 insertions(+), 10 deletions(-) diff --git a/src/eduid/webapp/idp/tests/test_api.py b/src/eduid/webapp/idp/tests/test_api.py index 69b707017..7a75ea325 100644 --- a/src/eduid/webapp/idp/tests/test_api.py +++ b/src/eduid/webapp/idp/tests/test_api.py @@ -3,16 +3,22 @@ import re from dataclasses import dataclass, field from pathlib import PurePath -from typing import Any, Mapping, Optional +from typing import Any, Mapping, Optional, Tuple +from unittest.mock import MagicMock, patch from bson import ObjectId +from fido2.webauthn import AuthenticatorAttachment from saml2 import BINDING_HTTP_POST, BINDING_HTTP_REDIRECT from saml2.client import Saml2Client from saml2.response import AuthnResponse from werkzeug.test import TestResponse from eduid.common.misc.timeutil import utc_now +from eduid.common.models.webauthn import WebauthnChallenge from eduid.userdb import ToUEvent +from eduid.userdb.credentials import Credential, FidoCredential, Webauthn +from eduid.userdb.credentials.external import TrustFramework, external_credential_from_dict +from eduid.userdb.idp import IdPUser from eduid.userdb.mail import MailAddress from eduid.userdb.user import User from eduid.webapp.common.api.testing import EduidAPITestCase @@ -21,7 +27,9 @@ from eduid.webapp.common.session.namespaces import AuthnRequestRef, PySAML2Dicts from eduid.webapp.idp.app import IdPApp, init_idp_app from eduid.webapp.idp.helpers import IdPAction +from eduid.webapp.idp.idp_authn import AuthnData from eduid.webapp.idp.sso_session import SSOSession, SSOSessionId +from eduid.webapp.idp.views.mfa_auth import CheckResult __author__ = "ft" @@ -50,6 +58,11 @@ class TouResult(GenericResult): pass +@dataclass +class MfaResult(GenericResult): + pass + + @dataclass class FinishedResultAPI(GenericResult): pass @@ -70,6 +83,7 @@ class LoginResultAPI: visit_order: list[IdPAction] = field(default_factory=list) pwauth_result: Optional[PwAuthResult] = None tou_result: Optional[TouResult] = None + mfa_result: Optional[MfaResult] = None finished_result: Optional[FinishedResultAPI] = None error: Optional[dict[str, Any]] = None @@ -133,6 +147,7 @@ def _try_login( assertion_consumer_service_url: Optional[str] = None, test_user: Optional[TestUser] = None, sso_cookie_val: Optional[str] = None, + mfa_credential: Optional[Credential] = None, ) -> LoginResultAPI: """ Try logging in to the IdP. @@ -203,8 +218,19 @@ def _try_login( cookie_jar.update(result.pwauth_result.cookies) if _action == IdPAction.MFA: - # Not implemented yet - return result + if mfa_credential is None: + assert user.eppn is not None # please mypy + _user = self.app.userdb.lookup_user(user.eppn) + assert _user is not None + # default mfa_credential to the first FidoCredential on the user + try: + mfa_credential = _user.credentials.filter(FidoCredential)[0] + except IndexError: + raise AssertionError( + f"No FidoCredential found for user {_user.eppn}, aborting with result {result}" + ) + + result.mfa_result = self._call_mfa(_next.payload["target"], ref, mfa_credential) if _action == IdPAction.TOU: result.tou_result = self._call_tou( @@ -275,6 +301,42 @@ def _call_tou(self, target: str, ref: str, user_accepts: Optional[str]) -> TouRe result = TouResult(payload=self.get_response_payload(response)) return result + @patch("eduid.webapp.idp.views.mfa_auth._check_webauthn") + @patch("eduid.webapp.common.authn.fido_tokens.start_token_verification") + def _call_mfa( + self, target: str, ref: str, mfa_credential: Credential, mock_stv: MagicMock, mock_cw: MagicMock + ) -> MfaResult: + mock_stv.return_value = WebauthnChallenge(webauthn_options="{'mock_webautn_options': 'mock_webauthn_options'}") + mock_cw.return_value = None + # first call to mfa endpoint returns a challenge + with self.session_cookie_anon(self.browser) as client: + with self.app.test_request_context(): + with client.session_transaction() as sess: + data = {"ref": ref, "csrf_token": sess.get_csrf_token()} + response = client.post(target, json=data) + + payload = self.get_response_payload(response=response) + assert ( + payload.get("webauthn_options") == mock_stv.return_value.webauthn_options + ), f"webauthn_options: {payload.get('webauthn_options')}, Expected: {mock_stv.return_value.webauthn_options}" + assert payload.get("finished") == False, "Expected finished=False" + + logger.debug(f"MFA endpoint returned (challenge):\n{json.dumps(response.json, indent=4)}") + + # mock valid mfa auth + mock_cw.return_value = CheckResult( + credential=mfa_credential, authn_data=AuthnData(cred_id=mfa_credential.key, timestamp=utc_now()) + ) + # second call to mfa endpoint returns a result + with self.session_cookie_anon(self.browser) as client: + with self.app.test_request_context(): + with client.session_transaction() as sess: + data = {"ref": ref, "csrf_token": sess.get_csrf_token()} + response = client.post(target, json=data) + + result = MfaResult(payload=self.get_response_payload(response)) + return result + @staticmethod def _extract_form_inputs(res: str) -> dict[str, Any]: inputs = {} @@ -316,12 +378,17 @@ def get_sso_session(self, sso_cookie_val: str) -> Optional[SSOSession]: return None return self.app.sso_sessions.get_session(SSOSessionId(sso_cookie_val)) - def add_test_user_tou(self, user: Optional[User] = None, version: Optional[str] = None) -> ToUEvent: + def add_test_user_tou(self, eppn: Optional[str] = None, version: Optional[str] = None) -> Tuple[IdPUser, ToUEvent]: """Utility function to add a valid ToU to the default test user""" if version is None: version = self.app.conf.tou_version - if user is None: - user = self.test_user + if eppn is None: + eppn = self.test_user.eppn + + # load user from central db to not get out of sync + user = self.app.userdb.lookup_user(eppn) + assert user is not None + tou = ToUEvent( version=version, created_by="idp_tests", @@ -330,13 +397,66 @@ def add_test_user_tou(self, user: Optional[User] = None, version: Optional[str] event_id=str(ObjectId()), ) user.tou.add(tou) - self.amdb.save(user) - return tou + self.request_user_sync(user) + return user, tou def add_test_user_mail_address(self, mail_address: MailAddress) -> None: """Utility function to add a mail address to the default test user""" - self.test_user.mail_addresses.add(mail_address) - self.amdb.save(self.test_user) + # load user from central db to not get out of sync + user = self.app.userdb.lookup_user(self.test_user.eppn) + assert user is not None + + user.mail_addresses.add(mail_address) + self.request_user_sync(user) + + def add_test_user_security_key( + self, + user: Optional[User] = None, + credential_id: Optional[str] = "webauthn_keyhandle", + is_verified: bool = False, + mfa_approved: bool = False, + credential: Optional[FidoCredential] = None, + ): + if user is None: + user = self.test_user + # load user from central db to not get out of sync + user = self.app.userdb.lookup_user(user.eppn) + assert user is not None + + if credential is None: + credential = Webauthn( + keyhandle=credential_id, + credential_data="test", + app_id="test", + description="test security key", + created_by="test", + authenticator=AuthenticatorAttachment.CROSS_PLATFORM, + is_verified=is_verified, + mfa_approved=mfa_approved, + ) + user.credentials.add(credential) + self.request_user_sync(user) + + def add_test_user_external_mfa_cred( + self, + user: Optional[User] = None, + trust_framework: Optional[TrustFramework] = None, + ): + if user is None: + user = self.test_user + # load user from central db to not get out of sync + user = self.app.userdb.lookup_user(user.eppn) + assert user is not None + + if trust_framework is None: + trust_framework = TrustFramework.SWECONN + + cred = external_credential_from_dict( + {"trust_framework": trust_framework, "created_ts": utc_now(), "created_by": "test"} + ) + assert cred is not None # please mypy + user.credentials.add(cred) + self.request_user_sync(user) def get_attributes(self, result, saml2_client: Optional[Saml2Client] = None): assert result.finished_result is not None From ed22f955fc8d1939ed6c6ad0237c68cb9c282b25 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Wed, 26 Jun 2024 10:55:24 +0200 Subject: [PATCH 03/11] refactor response checks --- src/eduid/webapp/idp/tests/test_SSO.py | 426 ++++++++++++++----------- 1 file changed, 236 insertions(+), 190 deletions(-) diff --git a/src/eduid/webapp/idp/tests/test_SSO.py b/src/eduid/webapp/idp/tests/test_SSO.py index c16ae82bb..881df75f7 100644 --- a/src/eduid/webapp/idp/tests/test_SSO.py +++ b/src/eduid/webapp/idp/tests/test_SSO.py @@ -1,6 +1,5 @@ #!/usr/bin/python -import datetime import logging from typing import Mapping, Optional, Sequence, Union from uuid import uuid4 @@ -18,7 +17,7 @@ from eduid.webapp.common.session import session from eduid.webapp.common.session.logindata import ExternalMfaData from eduid.webapp.common.session.namespaces import IdP_SAMLPendingRequest, RequestRef -from eduid.webapp.idp.assurance_data import EduidAuthnContextClass +from eduid.webapp.idp.assurance_data import EduidAuthnContextClass, SwamidAssurance from eduid.webapp.idp.helpers import IdPMsg from eduid.webapp.idp.idp_authn import AuthnData from eduid.webapp.idp.idp_saml import IdP_SAMLRequest, ServiceInfo @@ -174,6 +173,9 @@ def get_user_set_nins( proofing_method=proofing_method, ) user.identities.add(this_nin) + self.request_user_sync(user) + self.app.userdb.lookup_user(eppn) + assert user is not None # please mypy return user # ------------------------------------------------------------------------ @@ -188,9 +190,13 @@ def _get_login_response_authn( ) -> NextResult: if user is None: user = self.get_user_set_nins(self.test_user.eppn, []) + # load user from central db to not get out of sync + user = self.app.userdb.lookup_user(user.eppn) + assert user is not None if add_tou: - self.add_test_user_tou(user) + user, _ = self.add_test_user_tou(eppn=user.eppn) + assert user is not None sso_session_1 = SSOSession( authn_request_id="some-unique-id-1", @@ -223,7 +229,9 @@ def _get_login_response_authn( sso_session_1.add_authn_credential(data) # Need to save any changed credentials to the user - self.amdb.save(user) + self.request_user_sync(user) + user = self.app.userdb.lookup_user(user.eppn) + assert user is not None with self.app.test_request_context(): ticket = self._make_login_ticket(req_class_ref) @@ -240,6 +248,29 @@ def _get_login_response_authn( return login_next_step(ticket, sso_session_1) + @staticmethod + def _check_login_response_authn( + authn_result: NextResult, + message: IdPMsg, + expect_success: bool = True, + accr: Optional[EduidAuthnContextClass] = None, + assurance_profile: Optional[list[SwamidAssurance]] = None, + expect_error: Optional[bool] = False, + ): + assert authn_result.message == message, f"Message: {authn_result.message}, Expected: {message}" + if expect_success: + assert authn_result.authn_info + assert ( + authn_result.authn_info.class_ref == accr + ), f"class_ref: {authn_result.authn_info.class_ref}, Expected: {accr}" + if assurance_profile is not None: + assert authn_result.authn_info.authn_attributes["eduPersonAssurance"] == [ + item.value for item in assurance_profile + ], "assurance profile does not match" + else: + assert authn_result.authn_info is None, f"authn_info: {authn_result.authn_info}, Expected: None" + assert authn_result.error is expect_error, f"error: {authn_result.error}, Expected: {expect_error}" + # ------------------------------------------------------------------------ def test__get_login_response_1(self): @@ -248,19 +279,20 @@ def test__get_login_response_1(self): Expect the response Authn to be REFEDS MFA, and assurance attribute to include SWAMID AL3. """ - user = self.get_user_set_nins(self.test_user.eppn, ["190101011234"]) - user.credentials.add(_U2F_SWAMID_AL3) + self.get_user_set_nins(self.test_user.eppn, ["190101011234"]) + self.add_test_user_security_key(credential=_U2F_SWAMID_AL3) + user = self.app.userdb.get_user_by_eppn(self.test_user.eppn) out = self._get_login_response_authn( user=user, req_class_ref=EduidAuthnContextClass.REFEDS_MFA, credentials=["pw", _U2F_SWAMID_AL3], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_3 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_3, + ) def test__get_login_response_2(self): """ @@ -268,19 +300,20 @@ def test__get_login_response_2(self): Expect the response Authn to be REFEDS MFA. """ - user = self.get_user_set_nins(self.test_user.eppn, ["190101011234"]) - user.credentials.add(_U2F) + self.get_user_set_nins(self.test_user.eppn, ["190101011234"]) + self.add_test_user_security_key(credential=_U2F) + user = self.app.userdb.get_user_by_eppn(self.test_user.eppn) out = self._get_login_response_authn( user=user, req_class_ref=EduidAuthnContextClass.REFEDS_MFA, credentials=["pw", _U2F], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_2 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_2, + ) def test__get_login_response_external_multifactor(self): """ @@ -292,19 +325,19 @@ def test__get_login_response_external_multifactor(self): external_mfa = ExternalMfaData( issuer="issuer.example.com", authn_context="http://id.elegnamnden.se/loa/1.0/loa3", - timestamp=datetime.datetime.utcnow(), + timestamp=utc_now(), ) out = self._get_login_response_authn( user=user, req_class_ref=EduidAuthnContextClass.REFEDS_MFA, credentials=["pw", external_mfa], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_3 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_3, + ) def test__get_login_response_3(self): """ @@ -316,12 +349,12 @@ def test__get_login_response_3(self): req_class_ref=EduidAuthnContextClass.REFEDS_SFA, credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_SFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_SFA, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_response_4(self): """ @@ -333,12 +366,12 @@ def test__get_login_response_4(self): req_class_ref=EduidAuthnContextClass.REFEDS_SFA, credentials=["pw"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_SFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_SFA, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_response_UNSPECIFIED2(self): """ @@ -350,12 +383,12 @@ def test__get_login_response_UNSPECIFIED2(self): req_class_ref=EduidAuthnContextClass.REFEDS_SFA, credentials=["u2f"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_SFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_SFA, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_response_5(self): """ @@ -367,12 +400,12 @@ def test__get_login_response_5(self): req_class_ref=EduidAuthnContextClass.FIDO_U2F, credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.FIDO_U2F - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.FIDO_U2F, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_response_6(self): """ @@ -384,12 +417,12 @@ def test__get_login_response_6(self): req_class_ref=EduidAuthnContextClass.PASSWORD_PT, credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.PASSWORD_PT - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.PASSWORD_PT, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_response_7(self): """ @@ -401,12 +434,12 @@ def test__get_login_response_7(self): req_class_ref=EduidAuthnContextClass.PASSWORD_PT, credentials=["pw"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.PASSWORD_PT - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.PASSWORD_PT, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_response_8(self): """ @@ -418,8 +451,9 @@ def test__get_login_response_8(self): req_class_ref="urn:no-such-class", credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.assurance_failure, f"Wrong message: {out.message}" - assert out.authn_info is None + self._check_login_response_authn( + authn_result=out, message=IdPMsg.assurance_failure, expect_success=False, expect_error=True + ) def test__get_login_response_9(self): """ @@ -431,8 +465,9 @@ def test__get_login_response_9(self): req_class_ref="urn:no-such-class", credentials=["pw"], ) - assert out.message == IdPMsg.assurance_failure, f"Wrong message: {out.message}" - assert out.authn_info is None + self._check_login_response_authn( + authn_result=out, message=IdPMsg.assurance_failure, expect_success=False, expect_error=True + ) def test__get_login_response_10(self): """ @@ -444,12 +479,12 @@ def test__get_login_response_10(self): req_class_ref=None, credentials=["pw"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.PASSWORD_PT - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.PASSWORD_PT, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_response_11(self): """ @@ -461,12 +496,12 @@ def test__get_login_response_11(self): req_class_ref=None, credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_response_assurance_AL1(self): """ @@ -476,12 +511,12 @@ def test__get_login_response_assurance_AL1(self): req_class_ref=None, credentials=["pw"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.PASSWORD_PT - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.PASSWORD_PT, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_response_assurance_AL2(self): """ @@ -493,12 +528,12 @@ def test__get_login_response_assurance_AL2(self): user=user, credentials=["pw"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.PASSWORD_PT - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_2 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.PASSWORD_PT, + assurance_profile=self.app.conf.swamid_assurance_profile_2, + ) def test__get_login_eduid_mfa_fido_al1(self): """ @@ -510,12 +545,12 @@ def test__get_login_eduid_mfa_fido_al1(self): req_class_ref=EduidAuthnContextClass.EDUID_MFA, credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.EDUID_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.EDUID_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_refeds_mfa_fido_al1(self): """ @@ -527,12 +562,12 @@ def test__get_login_refeds_mfa_fido_al1(self): req_class_ref=EduidAuthnContextClass.REFEDS_MFA, credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_eduid_mfa_fido_al2(self): """ @@ -546,12 +581,12 @@ def test__get_login_eduid_mfa_fido_al2(self): req_class_ref=EduidAuthnContextClass.EDUID_MFA, credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.EDUID_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_2 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.EDUID_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_2, + ) def test__get_login_refeds_mfa_fido_al2(self): """ @@ -565,12 +600,12 @@ def test__get_login_refeds_mfa_fido_al2(self): req_class_ref=EduidAuthnContextClass.REFEDS_MFA, credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_2 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_2, + ) def test__get_login_eduid_mfa_fido_swamid_al2(self): """ @@ -585,12 +620,12 @@ def test__get_login_eduid_mfa_fido_swamid_al2(self): req_class_ref=EduidAuthnContextClass.EDUID_MFA, credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.EDUID_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_2 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.EDUID_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_2, + ) def test__get_login_eduid_mfa_fido_swamid_al3(self): """ @@ -598,19 +633,20 @@ def test__get_login_eduid_mfa_fido_swamid_al3(self): Expect the response Authn to be EDUID_MFA, eduPersonAssurance AL1,Al2 """ - user = self.get_user_set_nins(self.test_user.eppn, ["190101011234"]) - user.credentials.add(_U2F_SWAMID_AL3) + self.get_user_set_nins(self.test_user.eppn, ["190101011234"]) + self.add_test_user_security_key(credential=_U2F_SWAMID_AL3) + user = self.app.userdb.get_user_by_eppn(self.test_user.eppn) out = self._get_login_response_authn( user=user, req_class_ref=EduidAuthnContextClass.EDUID_MFA, credentials=["pw", _U2F_SWAMID_AL3], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.EDUID_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_3 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.EDUID_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_3, + ) def test__get_login_refeds_mfa_fido_swamid_al3(self): """ @@ -618,19 +654,20 @@ def test__get_login_refeds_mfa_fido_swamid_al3(self): Expect the response Authn to be REFEDS_MFA, eduPersonAssurance AL1,Al2,Al3 """ - user = self.get_user_set_nins(self.test_user.eppn, ["190101011234"]) - user.credentials.add(_U2F_SWAMID_AL3) + self.get_user_set_nins(self.test_user.eppn, ["190101011234"]) + self.add_test_user_security_key(credential=_U2F_SWAMID_AL3) + user = self.app.userdb.get_user_by_eppn(self.test_user.eppn) out = self._get_login_response_authn( user=user, req_class_ref=EduidAuthnContextClass.REFEDS_MFA, credentials=["pw", _U2F_SWAMID_AL3], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_3 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_3, + ) def test__get_login_eduid_mfa_external_mfa_al3(self): """ @@ -642,19 +679,19 @@ def test__get_login_eduid_mfa_external_mfa_al3(self): external_mfa = ExternalMfaData( issuer="issuer.example.com", authn_context="http://id.elegnamnden.se/loa/1.0/loa3", - timestamp=datetime.datetime.utcnow(), + timestamp=utc_now(), ) out = self._get_login_response_authn( user=user, req_class_ref=EduidAuthnContextClass.EDUID_MFA, credentials=["pw", external_mfa], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.EDUID_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_3 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.EDUID_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_3, + ) def test__get_login_refeds_mfa_external_mfa(self): """ @@ -666,19 +703,19 @@ def test__get_login_refeds_mfa_external_mfa(self): external_mfa = ExternalMfaData( issuer="issuer.example.com", authn_context="http://id.elegnamnden.se/loa/1.0/loa3", - timestamp=datetime.datetime.utcnow(), + timestamp=utc_now(), ) out = self._get_login_response_authn( user=user, req_class_ref=EduidAuthnContextClass.REFEDS_MFA, credentials=["pw", external_mfa], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_3 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_3, + ) def test__get_login_refeds_mfa_fido_al1_with_al3_mfa(self): """ @@ -686,19 +723,19 @@ def test__get_login_refeds_mfa_fido_al1_with_al3_mfa(self): Expect the response Authn to be REFEDS_MFA, eduPersonAssurance AL1 """ - user = self.app.central_userdb.get_user_by_eppn(self.test_user.eppn) + user = self.app.userdb.lookup_user(self.test_user.eppn) user.credentials.add(_U2F_SWAMID_AL3) self.app.central_userdb.save(user) out = self._get_login_response_authn( req_class_ref=EduidAuthnContextClass.REFEDS_MFA, credentials=["pw", _U2F_SWAMID_AL3], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.REFEDS_MFA - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_1 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.REFEDS_MFA, + assurance_profile=self.app.conf.swamid_assurance_profile_1, + ) def test__get_login_response_eduid_mfa_no_multifactor(self): """ @@ -707,8 +744,7 @@ def test__get_login_response_eduid_mfa_no_multifactor(self): This is not a failure, the user just needs to do MFA too. """ out = self._get_login_response_authn(req_class_ref=EduidAuthnContextClass.EDUID_MFA, credentials=["pw"]) - assert out.message == IdPMsg.mfa_required, f"Wrong message: {out.message}" - assert out.error is False + self._check_login_response_authn(authn_result=out, message=IdPMsg.mfa_required, expect_success=False) def test__get_login_response_refeds_mfa_no_multifactor(self): """ @@ -717,8 +753,7 @@ def test__get_login_response_refeds_mfa_no_multifactor(self): This is not a failure, the user just needs to do MFA too. """ out = self._get_login_response_authn(req_class_ref=EduidAuthnContextClass.REFEDS_MFA, credentials=["pw"]) - assert out.message == IdPMsg.mfa_required, f"Wrong message: {out.message}" - assert out.error is False + self._check_login_response_authn(authn_result=out, message=IdPMsg.mfa_required, expect_success=False) def test__get_login_digg_loa2_fido_mfa(self): """ @@ -726,21 +761,20 @@ def test__get_login_digg_loa2_fido_mfa(self): Expect the response Authn to be DIGG_LOA2. """ - user = self.get_user_set_nins( - self.test_user.eppn, ["190101011234"], proofing_method=IdentityProofingMethod.BANKID - ) - user.credentials.add(_U2F_SWAMID_AL3) + self.get_user_set_nins(self.test_user.eppn, ["190101011234"], proofing_method=IdentityProofingMethod.BANKID) + self.add_test_user_security_key(credential=_U2F_SWAMID_AL3) + user = self.app.userdb.get_user_by_eppn(self.test_user.eppn) out = self._get_login_response_authn( user=user, req_class_ref=EduidAuthnContextClass.DIGG_LOA2, credentials=["pw", _U2F_SWAMID_AL3], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.DIGG_LOA2 - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_3 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.DIGG_LOA2, + assurance_profile=self.app.conf.swamid_assurance_profile_3, + ) def test__get_login_digg_loa2_fido_mfa_no_identity_proofing_method(self): """ @@ -760,12 +794,12 @@ def test__get_login_digg_loa2_fido_mfa_no_identity_proofing_method(self): req_class_ref=EduidAuthnContextClass.DIGG_LOA2, credentials=["pw", _U2F_SWAMID_AL3], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.DIGG_LOA2 - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_3 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.DIGG_LOA2, + assurance_profile=self.app.conf.swamid_assurance_profile_3, + ) # test with not allowed identity proofing methods for nin_verified_by in ["lookup_mobile_proofing", "oidc_proofing"]: @@ -775,8 +809,12 @@ def test__get_login_digg_loa2_fido_mfa_no_identity_proofing_method(self): req_class_ref=EduidAuthnContextClass.DIGG_LOA2, credentials=["pw", _U2F_SWAMID_AL3], ) - assert out.message == IdPMsg.identity_proofing_method_not_allowed, f"Wrong message: {out.message}" - assert out.authn_info is None + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.identity_proofing_method_not_allowed, + expect_success=False, + expect_error=True, + ) def test__get_login_digg_loa2_external_mfa(self): """ @@ -797,12 +835,12 @@ def test__get_login_digg_loa2_external_mfa(self): req_class_ref=EduidAuthnContextClass.DIGG_LOA2, credentials=["pw", external_mfa], ) - assert out.message == IdPMsg.proceed, f"Wrong message: {out.message}" - assert out.authn_info - assert out.authn_info.class_ref == EduidAuthnContextClass.DIGG_LOA2 - assert out.authn_info.authn_attributes["eduPersonAssurance"] == [ - item.value for item in self.app.conf.swamid_assurance_profile_3 - ] + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.proceed, + accr=EduidAuthnContextClass.DIGG_LOA2, + assurance_profile=self.app.conf.swamid_assurance_profile_3, + ) def test__get_login_digg_loa2_identity_proofing_method_not_allowed(self): """ @@ -823,8 +861,12 @@ def test__get_login_digg_loa2_identity_proofing_method_not_allowed(self): req_class_ref=EduidAuthnContextClass.DIGG_LOA2, credentials=["pw", external_mfa], ) - assert out.message == IdPMsg.identity_proofing_method_not_allowed, f"Wrong message: {out.message}" - assert out.error is True + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.identity_proofing_method_not_allowed, + expect_success=False, + expect_error=True, + ) def test__get_login_digg_loa2_mfa_proofing_method_not_allowed(self): """ @@ -840,8 +882,12 @@ def test__get_login_digg_loa2_mfa_proofing_method_not_allowed(self): req_class_ref=EduidAuthnContextClass.DIGG_LOA2, credentials=["pw", "u2f"], ) - assert out.message == IdPMsg.mfa_proofing_method_not_allowed, f"Wrong message: {out.message}" - assert out.error is True + self._check_login_response_authn( + authn_result=out, + message=IdPMsg.mfa_proofing_method_not_allowed, + expect_success=False, + expect_error=True, + ) def test__forceauthn_request(self): """ForceAuthn can apparently be either 'true' or '1'. From 5b028f90e07b79ed82a14f8b8ded398fd34b4306 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Wed, 26 Jun 2024 11:13:13 +0200 Subject: [PATCH 04/11] more descriptive argument for a user setting --- src/eduid/webapp/idp/tests/test_api.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/eduid/webapp/idp/tests/test_api.py b/src/eduid/webapp/idp/tests/test_api.py index 7a75ea325..5c8c6ecca 100644 --- a/src/eduid/webapp/idp/tests/test_api.py +++ b/src/eduid/webapp/idp/tests/test_api.py @@ -416,6 +416,7 @@ def add_test_user_security_key( is_verified: bool = False, mfa_approved: bool = False, credential: Optional[FidoCredential] = None, + force_mfa_user_setting: bool = True, ): if user is None: user = self.test_user @@ -435,6 +436,7 @@ def add_test_user_security_key( mfa_approved=mfa_approved, ) user.credentials.add(credential) + user.settings.force_mfa = force_mfa_user_setting self.request_user_sync(user) def add_test_user_external_mfa_cred( From cd64a20ab75be5ba30d3901737348233278e4cff Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Wed, 26 Jun 2024 11:13:45 +0200 Subject: [PATCH 05/11] add tests for mfa logins --- src/eduid/webapp/idp/tests/test_login.py | 105 +++++++++++++++++++++-- 1 file changed, 100 insertions(+), 5 deletions(-) diff --git a/src/eduid/webapp/idp/tests/test_login.py b/src/eduid/webapp/idp/tests/test_login.py index 4d21bdb61..66314c364 100644 --- a/src/eduid/webapp/idp/tests/test_login.py +++ b/src/eduid/webapp/idp/tests/test_login.py @@ -28,6 +28,7 @@ class IdPTestLoginAPI(IdPAPITests): + def test_login_start(self) -> None: result = self._try_login(test_user=TestUser(eppn=None, password=None)) assert result.visit_order == [IdPAction.PWAUTH] @@ -82,13 +83,101 @@ def test_login_pwauth_right_password_and_tou_acceptance(self) -> None: assert "eduPersonPrincipalName" in attributes assert attributes["eduPersonPrincipalName"] == [f"hubba-bubba@{self.app.conf.default_eppn_scope}"] - def test_login_missing_attributes(self) -> None: + def test_login_mfaauth(self) -> None: + # pre-accept ToU for this test + self.add_test_user_tou() + + # add security key to user + self.add_test_user_security_key() + + # Patch the VCCSClient, so we do not need a vccs server + with patch.object(VCCSClient, "authenticate") as mock_vccs: + mock_vccs.return_value = True + result = self._try_login() + + assert result.visit_order == [ + IdPAction.PWAUTH, + IdPAction.MFA, + IdPAction.FINISHED, + ], f"NOT expected visit order {result.visit_order}" + assert result.sso_cookie_val is not None + assert result.finished_result is not None + assert result.finished_result.payload["message"] == IdPMsg.finished.value + assert result.finished_result.payload["target"] == "https://sp.example.edu/saml2/acs/" + assert result.finished_result.payload["parameters"]["RelayState"] == self.relay_state + + attributes = self.get_attributes(result) + + assert "eduPersonPrincipalName" in attributes + assert attributes["eduPersonPrincipalName"] == [f"hubba-bubba@{self.app.conf.default_eppn_scope}"] + + def test_login_no_mandatory_mfa(self) -> None: # pre-accept ToU for this test self.add_test_user_tou() + # add security key to user + self.add_test_user_security_key(force_mfa_user_setting=False) + + # Patch the VCCSClient, so we do not need a vccs server + with patch.object(VCCSClient, "authenticate") as mock_vccs: + mock_vccs.return_value = True + result = self._try_login() + + assert result.visit_order == [ + IdPAction.PWAUTH, + IdPAction.FINISHED, + ], f"NOT expected visit order {result.visit_order}" + assert result.sso_cookie_val is not None + assert result.finished_result is not None + assert result.finished_result.payload["message"] == IdPMsg.finished.value + assert result.finished_result.payload["target"] == "https://sp.example.edu/saml2/acs/" + assert result.finished_result.payload["parameters"]["RelayState"] == self.relay_state + + attributes = self.get_attributes(result) + + assert "eduPersonPrincipalName" in attributes + assert attributes["eduPersonPrincipalName"] == [f"hubba-bubba@{self.app.conf.default_eppn_scope}"] + + def test_login_no_mandatory_mfa_with_mfa_accr(self) -> None: + # pre-accept ToU for this test + self.add_test_user_tou() + + # add security key to user + self.add_test_user_security_key(force_mfa_user_setting=False) + + # Patch the VCCSClient, so we do not need a vccs server + with patch.object(VCCSClient, "authenticate") as mock_vccs: + mock_vccs.return_value = True + result = self._try_login( + authn_context={ + "authn_context_class_ref": [EduidAuthnContextClass.REFEDS_MFA.value], + "comparison": "exact", + } + ) + + assert result.visit_order == [ + IdPAction.PWAUTH, + IdPAction.MFA, + IdPAction.FINISHED, + ], f"NOT expected visit order {result.visit_order}" + assert result.sso_cookie_val is not None + assert result.finished_result is not None + assert result.finished_result.payload["message"] == IdPMsg.finished.value + assert result.finished_result.payload["target"] == "https://sp.example.edu/saml2/acs/" + assert result.finished_result.payload["parameters"]["RelayState"] == self.relay_state + + attributes = self.get_attributes(result) + + assert "eduPersonPrincipalName" in attributes + assert attributes["eduPersonPrincipalName"] == [f"hubba-bubba@{self.app.conf.default_eppn_scope}"] + + def test_login_missing_attributes(self) -> None: + # pre-accept ToU for this test + user, _ = self.add_test_user_tou() + # remove mail address from user to simulate missing attribute - self.test_user.mail_addresses = MailAddressList() - self.request_user_sync(self.test_user) + user.mail_addresses = MailAddressList() + self.request_user_sync(user) # Patch the VCCSClient, so we do not need a vccs server with patch.object(VCCSClient, "authenticate") as mock_vccs: @@ -104,6 +193,9 @@ def test_login_missing_attributes(self) -> None: assert attributes["mailLocalAddress"] == [] def test_ForceAuthn_with_existing_SSO_session(self) -> None: + # add security key to user + self.add_test_user_security_key() + for accr in [None, EduidAuthnContextClass.PASSWORD_PT, EduidAuthnContextClass.REFEDS_MFA]: requested_authn_context = None if accr is not None: @@ -134,8 +226,11 @@ def test_ForceAuthn_with_existing_SSO_session(self) -> None: ) if accr is EduidAuthnContextClass.REFEDS_MFA: - # we currently have no way to mock a correct MFA authentication so just check that we try to do MFA - assert result2.visit_order == [IdPAction.PWAUTH, IdPAction.MFA] + assert result2.visit_order == [ + IdPAction.PWAUTH, + IdPAction.MFA, + IdPAction.FINISHED, + ], f"Actual visit order: {result2.visit_order}" else: assert result2.finished_result is not None authn_response2 = self.parse_saml_authn_response(result2.finished_result) From 4f2ba8f067108f821f5cef8c19738a8872f7144c Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Wed, 26 Jun 2024 11:13:57 +0200 Subject: [PATCH 06/11] nitpicking --- src/eduid/webapp/idp/views/mfa_auth.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/eduid/webapp/idp/views/mfa_auth.py b/src/eduid/webapp/idp/views/mfa_auth.py index 9ea1e45f4..7fae95a9e 100644 --- a/src/eduid/webapp/idp/views/mfa_auth.py +++ b/src/eduid/webapp/idp/views/mfa_auth.py @@ -45,7 +45,7 @@ def mfa_auth( return error_response(message=IdPMsg.not_available) if not sso_session: - current_app.logger.error(f"MFA auth called without an SSO session") + current_app.logger.error("MFA auth called without an SSO session") return error_response(message=IdPMsg.no_sso_session) user = lookup_user(sso_session.eppn) @@ -191,7 +191,7 @@ def _check_webauthn( # Process webauthn_response # if not mfa_action.webauthn_state: - current_app.logger.error(f"No active webauthn challenge found in the session, can't do verification") + current_app.logger.error("No active webauthn challenge found in the session, can't do verification") return CheckResult(response=error_response(message=IdPMsg.general_failure)) try: From 6cf164541e50a53b50e7fa0585f1af98e39c1fdb Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Thu, 27 Jun 2024 10:26:33 +0200 Subject: [PATCH 07/11] refactor login tests to use shared _check_login_result --- src/eduid/webapp/idp/tests/test_api.py | 43 +++- src/eduid/webapp/idp/tests/test_login.py | 245 ++++++++++++++--------- 2 files changed, 195 insertions(+), 93 deletions(-) diff --git a/src/eduid/webapp/idp/tests/test_api.py b/src/eduid/webapp/idp/tests/test_api.py index 5c8c6ecca..e56206a1a 100644 --- a/src/eduid/webapp/idp/tests/test_api.py +++ b/src/eduid/webapp/idp/tests/test_api.py @@ -3,7 +3,7 @@ import re from dataclasses import dataclass, field from pathlib import PurePath -from typing import Any, Mapping, Optional, Tuple +from typing import Any, Mapping, Optional, Tuple, Union from unittest.mock import MagicMock, patch from bson import ObjectId @@ -466,3 +466,44 @@ def get_attributes(self, result, saml2_client: Optional[Saml2Client] = None): session_info = authn_response.session_info() attributes: dict[str, list[Any]] = session_info["ava"] return attributes + + def _assert_dict_contains(self, actual: dict[str, Any], expected: dict[str, Any]): + # try: + for key, value in expected.items(): + assert key in actual, f"expected {key} not in {actual}" + if isinstance(value, dict): + self._assert_dict_contains(actual[key], value) + else: + assert actual[key] == value, f"expected {key} value: {actual[key]} != {value} in {actual}" + # except AssertionError as e: + # raise AssertionError(f"{e}\n\nactual: {actual}") + + def _check_login_result( + self, + result: LoginResultAPI, + visit_order: list[IdPAction], + sso_cookie_val: Optional[Union[str, bool]] = True, + finish_result: Optional[FinishedResultAPI] = None, + pwauth_result: Optional[PwAuthResult] = None, + error: Optional[dict[str, Any]] = None, + ): + assert result.visit_order == visit_order, f"visit_order: {result.visit_order}, expected: {visit_order}" + + if sso_cookie_val is True: + assert result.sso_cookie_val is not None, "Expected sso_cookie_val but it is None" + else: + assert ( + result.sso_cookie_val == sso_cookie_val + ), f"sso_cookie_val: {result.sso_cookie_val}, expected: {sso_cookie_val}" + + if finish_result is not None: + assert result.finished_result is not None, "Expected finished_result but it is None" + self._assert_dict_contains(result.finished_result.payload, finish_result.payload) + + if pwauth_result is not None: + assert result.pwauth_result is not None, "Expected pwauth_result but it is None" + self._assert_dict_contains(result.pwauth_result.payload, pwauth_result.payload) + + if error is not None: + assert result.error is not None, "Expected error but it is None" + self._assert_dict_contains(result.error, error) diff --git a/src/eduid/webapp/idp/tests/test_login.py b/src/eduid/webapp/idp/tests/test_login.py index e028c661c..e824e3a19 100644 --- a/src/eduid/webapp/idp/tests/test_login.py +++ b/src/eduid/webapp/idp/tests/test_login.py @@ -11,15 +11,15 @@ from eduid.common.misc.timeutil import utc_now from eduid.common.models.generic import HttpUrlStr +from eduid.common.models.saml2 import EduidAuthnContextClass from eduid.userdb import MailAddress from eduid.userdb.credentials import Password from eduid.userdb.maccapi.userdb import ManagedAccount from eduid.userdb.mail import MailAddressList from eduid.vccs.client import VCCSClient -from eduid.common.models.saml2 import EduidAuthnContextClass from eduid.webapp.common.authn.utils import get_saml2_config from eduid.webapp.idp.helpers import IdPAction, IdPMsg -from eduid.webapp.idp.tests.test_api import IdPAPITests, TestUser +from eduid.webapp.idp.tests.test_api import FinishedResultAPI, IdPAPITests, PwAuthResult, TestUser from eduid.workers.am import AmCelerySingleton logger = logging.getLogger(__name__) @@ -154,20 +154,23 @@ def test_login_no_mandatory_mfa_with_mfa_accr(self) -> None: "comparison": "exact", } ) - - assert result.visit_order == [ - IdPAction.PWAUTH, - IdPAction.MFA, - IdPAction.FINISHED, - ], f"NOT expected visit order {result.visit_order}" - assert result.sso_cookie_val is not None - assert result.finished_result is not None - assert result.finished_result.payload["message"] == IdPMsg.finished.value - assert result.finished_result.payload["target"] == "https://sp.example.edu/saml2/acs/" - assert result.finished_result.payload["parameters"]["RelayState"] == self.relay_state + self._check_login_result( + result=result, + visit_order=[ + IdPAction.PWAUTH, + IdPAction.MFA, + IdPAction.FINISHED, + ], + finish_result=FinishedResultAPI( + payload={ + "message": IdPMsg.finished.value, + "target": "https://sp.example.edu/saml2/acs/", + "parameters": {"RelayState": self.relay_state}, + } + ), + ) attributes = self.get_attributes(result) - assert "eduPersonPrincipalName" in attributes assert attributes["eduPersonPrincipalName"] == [f"hubba-bubba@{self.app.conf.default_eppn_scope}"] @@ -184,10 +187,17 @@ def test_login_missing_attributes(self) -> None: mock_vccs.return_value = True result = self._try_login() - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] - assert result.finished_result is not None - assert len(result.finished_result.payload["missing_attributes"]) == 1 - assert result.finished_result.payload["missing_attributes"][0]["friendly_name"] == "mailLocalAddress" + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI( + payload={ + "missing_attributes": [ + {"friendly_name": "mailLocalAddress", "name": "urn:oid:2.16.840.1.113730.3.1.13"} + ] + } + ), + ) attributes = self.get_attributes(result) assert attributes["mailLocalAddress"] == [] @@ -246,11 +256,12 @@ def test_terminated_user(self) -> None: with patch.object(VCCSClient, "authenticate") as mock_vccs: mock_vccs.return_value = True result = self._try_login() - assert result.visit_order == [IdPAction.PWAUTH] - assert result.error is not None - payload = result.error.get("payload") - assert payload is not None - assert payload.get("message") == IdPMsg.user_terminated.value + + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH], + error={"payload": {"message": IdPMsg.user_terminated.value}}, + ) def test_with_unknown_sp(self) -> None: sp_config = get_saml2_config(self.app.conf.pysaml2_config, name="UNKNOWN_SP_CONFIG") @@ -264,11 +275,11 @@ def test_with_unknown_sp(self) -> None: mock_vccs.return_value = True result = self._try_login(saml2_client=saml2_client) - assert result.visit_order == [IdPAction.PWAUTH] - assert result.error is not None - assert result.error.get("status_code") == 400 - assert result.error.get("status") == "400 BAD REQUEST" - assert result.error.get("message") == "SAML error: Unknown Service Provider" + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH], + error={"status_code": 400, "status": "400 BAD REQUEST", "message": "SAML error: Unknown Service Provider"}, + ) def test_sso_to_unknown_sp(self) -> None: # pre-accept ToU for this test @@ -279,18 +290,23 @@ def test_sso_to_unknown_sp(self) -> None: mock_vccs.return_value = True result = self._try_login() - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + ) sp_config = get_saml2_config(self.app.conf.pysaml2_config, name="UNKNOWN_SP_CONFIG") saml2_client = Saml2Client(config=sp_config) # Don't patch VCCS here to ensure a SSO is done, not a password authentication result2 = self._try_login(saml2_client=saml2_client) - assert result2.visit_order == [] - assert result2.error is not None - assert result2.error.get("status_code") == 400 - assert result2.error.get("status") == "400 BAD REQUEST" - assert result2.error.get("message") == "SAML error: Unknown Service Provider" + + self._check_login_result( + result=result2, + visit_order=[], + sso_cookie_val=None, + error={"status_code": 400, "status": "400 BAD REQUEST", "message": "SAML error: Unknown Service Provider"}, + ) def test_eduperson_targeted_id(self) -> None: sp_config = get_saml2_config(self.app.conf.pysaml2_config, name="COCO_SP_CONFIG") @@ -304,12 +320,13 @@ def test_eduperson_targeted_id(self) -> None: mock_vccs.return_value = True result = self._try_login(saml2_client=saml2_client) - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) - assert result.finished_result is not None attributes = self.get_attributes(result, saml2_client=saml2_client) - - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] assert "eduPersonTargetedID" in attributes assert attributes["eduPersonTargetedID"] == ["71a13b105e83aa69c31f41b08ea83694e0fae5f368d17ef18ba59e0f9e407ec9"] @@ -325,7 +342,11 @@ def test_schac_personal_unique_code_esi(self) -> None: mock_vccs.return_value = True result = self._try_login(saml2_client=saml2_client) - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) attributes = self.get_attributes(result, saml2_client=saml2_client) @@ -350,9 +371,11 @@ def test_pairwise_id(self) -> None: mock_vccs.return_value = True result = self._try_login(saml2_client=saml2_client) - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] - - assert result.finished_result is not None + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) attributes = self.get_attributes(result, saml2_client=saml2_client) @@ -372,7 +395,11 @@ def test_subject_id(self) -> None: mock_vccs.return_value = True result = self._try_login(saml2_client=saml2_client) - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) attributes = self.get_attributes(result, saml2_client=saml2_client) assert attributes["subject-id"] == ["hubba-bubba@test.scope"] @@ -392,7 +419,11 @@ def test_mail_local_address(self) -> None: mock_vccs.return_value = True result = self._try_login(saml2_client=saml2_client) - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) attributes = self.get_attributes(result, saml2_client=saml2_client) @@ -407,9 +438,11 @@ def test_successful_authentication_alternative_acs(self) -> None: mock_vccs.return_value = True result = self._try_login(assertion_consumer_service_url="https://localhost:8080/acs/") - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] - assert result.finished_result is not None - assert result.finished_result.payload["target"] == "https://localhost:8080/acs/" + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"target": "https://localhost:8080/acs/"}), + ) def test_geo_statistics_success(self) -> None: # pre-accept ToU for this test @@ -444,8 +477,11 @@ def test_geo_statistics_success(self) -> None: } } - assert result.finished_result is not None - assert result.finished_result.payload["message"] == IdPMsg.finished.value + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) def test_geo_statistics_fail(self) -> None: # pre-accept ToU for this test @@ -463,8 +499,11 @@ def test_geo_statistics_fail(self) -> None: result = self._try_login() assert mock_post.call_count == 1 - assert result.finished_result is not None - assert result.finished_result.payload["message"] == IdPMsg.finished.value + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) class IdPTestLoginAPIManagedAccounts(IdPAPITests): @@ -492,10 +531,12 @@ def _create_managed_account_user(self, eppn: str) -> ManagedAccount: def test_login_pwauth_wrong_password(self) -> None: result = self._try_login() - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.PWAUTH] - assert result.sso_cookie_val is None - assert result.pwauth_result is not None - assert result.pwauth_result.payload["message"] == IdPMsg.wrong_credentials.value + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.PWAUTH], + sso_cookie_val=None, + pwauth_result=PwAuthResult(payload={"message": IdPMsg.wrong_credentials.value}), + ) def test_login_pwauth_right_password(self) -> None: # Patch the VCCSClient, so we do not need a vccs server @@ -503,12 +544,17 @@ def test_login_pwauth_right_password(self) -> None: mock_vccs.return_value = True result = self._try_login() - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] - assert result.sso_cookie_val is not None - assert result.finished_result is not None - assert result.finished_result.payload["message"] == IdPMsg.finished.value - assert result.finished_result.payload["target"] == "https://sp.example.edu/saml2/acs/" - assert result.finished_result.payload["parameters"]["RelayState"] == self.relay_state + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI( + payload={ + "message": IdPMsg.finished.value, + "target": "https://sp.example.edu/saml2/acs/", + "parameters": {"RelayState": self.relay_state}, + } + ), + ) attributes = self.get_attributes(result) @@ -549,11 +595,10 @@ def test_terminated_user(self) -> None: with patch.object(VCCSClient, "authenticate") as mock_vccs: mock_vccs.return_value = True result = self._try_login() - assert result.visit_order == [IdPAction.PWAUTH] - assert result.error is not None - payload = result.error.get("payload") - assert payload is not None - assert payload.get("message") == IdPMsg.user_terminated.value + + self._check_login_result( + result=result, visit_order=[IdPAction.PWAUTH], error={"payload": {"message": IdPMsg.user_terminated.value}} + ) def test_with_unknown_sp(self) -> None: sp_config = get_saml2_config(self.app.conf.pysaml2_config, name="UNKNOWN_SP_CONFIG") @@ -564,11 +609,11 @@ def test_with_unknown_sp(self) -> None: mock_vccs.return_value = True result = self._try_login(saml2_client=saml2_client) - assert result.visit_order == [IdPAction.PWAUTH] - assert result.error is not None - assert result.error.get("status_code") == 400 - assert result.error.get("status") == "400 BAD REQUEST" - assert result.error.get("message") == "SAML error: Unknown Service Provider" + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH], + error={"status_code": 400, "status": "400 BAD REQUEST", "message": "SAML error: Unknown Service Provider"}, + ) def test_sso_to_unknown_sp(self) -> None: # Patch the VCCSClient, so we do not need a vccs server @@ -583,11 +628,13 @@ def test_sso_to_unknown_sp(self) -> None: # Don't patch VCCS here to ensure a SSO is done, not a password authentication result2 = self._try_login(saml2_client=saml2_client) - assert result2.visit_order == [] - assert result2.error is not None - assert result2.error.get("status_code") == 400 - assert result2.error.get("status") == "400 BAD REQUEST" - assert result2.error.get("message") == "SAML error: Unknown Service Provider" + + self._check_login_result( + result=result2, + visit_order=[], + sso_cookie_val=None, + error={"status_code": 400, "status": "400 BAD REQUEST", "message": "SAML error: Unknown Service Provider"}, + ) def test_eduperson_targeted_id(self) -> None: sp_config = get_saml2_config(self.app.conf.pysaml2_config, name="COCO_SP_CONFIG") @@ -601,12 +648,13 @@ def test_eduperson_targeted_id(self) -> None: mock_vccs.return_value = True result = self._try_login(saml2_client=saml2_client) - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) - assert result.finished_result is not None attributes = self.get_attributes(result, saml2_client=saml2_client) - - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] assert "eduPersonTargetedID" in attributes assert attributes["eduPersonTargetedID"] == ["f0e831c0fcc8d61aef72e92f34e51f415f101050b8291a8c2c41ab4978b18f93"] @@ -622,12 +670,13 @@ def test_pairwise_id(self) -> None: mock_vccs.return_value = True result = self._try_login(saml2_client=saml2_client) - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] - - assert result.finished_result is not None + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) attributes = self.get_attributes(result, saml2_client=saml2_client) - assert attributes["pairwise-id"] == [ "133d9ecc64c5d8ed99ef00329e87b8677e74fc573e3f41ba0c259695813b9c19@test.scope" ] @@ -644,7 +693,11 @@ def test_subject_id(self) -> None: mock_vccs.return_value = True result = self._try_login(saml2_client=saml2_client) - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) attributes = self.get_attributes(result, saml2_client=saml2_client) assert attributes["subject-id"] == [f"{self.test_eppn}@test.scope"] @@ -658,9 +711,11 @@ def test_successful_authentication_alternative_acs(self) -> None: mock_vccs.return_value = True result = self._try_login(assertion_consumer_service_url="https://localhost:8080/acs/") - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] - assert result.finished_result is not None - assert result.finished_result.payload["target"] == "https://localhost:8080/acs/" + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"target": "https://localhost:8080/acs/"}), + ) def test_geo_statistics_success(self) -> None: # enable geo statistics @@ -692,8 +747,11 @@ def test_geo_statistics_success(self) -> None: } } - assert result.finished_result is not None - assert result.finished_result.payload["message"] == IdPMsg.finished.value + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) def test_geo_statistics_fail(self) -> None: # enable geo statistics @@ -708,5 +766,8 @@ def test_geo_statistics_fail(self) -> None: result = self._try_login() assert mock_post.call_count == 1 - assert result.finished_result is not None - assert result.finished_result.payload["message"] == IdPMsg.finished.value + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI(payload={"message": IdPMsg.finished.value}), + ) From 9a1fce80ce5619352d6d8a22cd11d33fb2018628 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Thu, 27 Jun 2024 10:27:07 +0200 Subject: [PATCH 08/11] make reformat --- src/eduid/common/models/saml2.py | 2 +- src/eduid/userdb/idp/user.py | 2 +- src/eduid/userdb/tests/test_idp_user.py | 2 +- src/eduid/webapp/authn/tests/test_authn.py | 2 +- src/eduid/webapp/common/authn/utils.py | 2 +- src/eduid/webapp/common/session/tests/test_namespaces.py | 1 + src/eduid/webapp/idp/assurance.py | 2 +- src/eduid/webapp/idp/assurance_data.py | 2 +- src/eduid/webapp/idp/other_device/db.py | 2 +- 9 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/eduid/common/models/saml2.py b/src/eduid/common/models/saml2.py index 08c3e5296..f69065dd5 100644 --- a/src/eduid/common/models/saml2.py +++ b/src/eduid/common/models/saml2.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -from enum import unique, Enum +from enum import Enum, unique __author__ = "lundberg" diff --git a/src/eduid/userdb/idp/user.py b/src/eduid/userdb/idp/user.py index de9f1f344..f2f80917b 100644 --- a/src/eduid/userdb/idp/user.py +++ b/src/eduid/userdb/idp/user.py @@ -7,8 +7,8 @@ from enum import Enum, unique from typing import Any, Optional -from eduid.userdb import User from eduid.common.models.saml2 import EduidAuthnContextClass +from eduid.userdb import User logger = logging.getLogger(__name__) diff --git a/src/eduid/userdb/tests/test_idp_user.py b/src/eduid/userdb/tests/test_idp_user.py index 804fcf2a1..b8e0b9ec0 100644 --- a/src/eduid/userdb/tests/test_idp_user.py +++ b/src/eduid/userdb/tests/test_idp_user.py @@ -1,9 +1,9 @@ from unittest import TestCase +from eduid.common.models.saml2 import EduidAuthnContextClass from eduid.common.testing_base import normalised_data from eduid.userdb.fixtures.users import UserFixtures from eduid.userdb.idp.user import SUPPORTED_SAML_ATTRIBUTES, IdPUser, SAMLAttributeSettings -from eduid.common.models.saml2 import EduidAuthnContextClass __author__ = "lundberg" diff --git a/src/eduid/webapp/authn/tests/test_authn.py b/src/eduid/webapp/authn/tests/test_authn.py index d11f315fc..0366380de 100644 --- a/src/eduid/webapp/authn/tests/test_authn.py +++ b/src/eduid/webapp/authn/tests/test_authn.py @@ -15,6 +15,7 @@ from eduid.common.config.base import FrontendAction from eduid.common.config.parsers import load_config from eduid.common.misc.timeutil import utc_now +from eduid.common.models.saml2 import EduidAuthnContextClass from eduid.common.utils import urlappend from eduid.webapp.authn.app import AuthnApp, authn_init_app from eduid.webapp.authn.settings.common import AuthnConfig @@ -22,7 +23,6 @@ from eduid.webapp.common.authn.acs_enums import AuthnAcsAction from eduid.webapp.common.authn.cache import OutstandingQueriesCache from eduid.webapp.common.authn.eduid_saml2 import get_authn_request -from eduid.common.models.saml2 import EduidAuthnContextClass from eduid.webapp.common.authn.middleware import AuthnBaseApp from eduid.webapp.common.authn.tests.responses import auth_response, logout_request, logout_response from eduid.webapp.common.authn.utils import get_location, no_authn_views diff --git a/src/eduid/webapp/common/authn/utils.py b/src/eduid/webapp/common/authn/utils.py index 3dc328c99..496329da0 100644 --- a/src/eduid/webapp/common/authn/utils.py +++ b/src/eduid/webapp/common/authn/utils.py @@ -8,7 +8,7 @@ from saml2.config import SPConfig from saml2.typing import SAMLHttpArgs -from eduid.common.config.base import EduIDBaseAppConfig, FrontendAction, FrontendActionMixin, AuthnParameters +from eduid.common.config.base import AuthnParameters, EduIDBaseAppConfig, FrontendAction, FrontendActionMixin from eduid.common.config.exceptions import BadConfiguration from eduid.common.misc.timeutil import utc_now from eduid.common.utils import urlappend diff --git a/src/eduid/webapp/common/session/tests/test_namespaces.py b/src/eduid/webapp/common/session/tests/test_namespaces.py index 30602330f..53ddc80d4 100644 --- a/src/eduid/webapp/common/session/tests/test_namespaces.py +++ b/src/eduid/webapp/common/session/tests/test_namespaces.py @@ -123,6 +123,7 @@ def test_clear_namespace(self): assert third.signup.email.address == "test@example.com" assert third.signup.email.verification_code is None + class TestAuthnNamespace(EduidAPITestCase): app: SessionTestApp diff --git a/src/eduid/webapp/idp/assurance.py b/src/eduid/webapp/idp/assurance.py index d12f9ddc0..d9215beb7 100644 --- a/src/eduid/webapp/idp/assurance.py +++ b/src/eduid/webapp/idp/assurance.py @@ -3,11 +3,11 @@ import logging from eduid.common.misc.timeutil import utc_now +from eduid.common.models.saml2 import EduidAuthnContextClass from eduid.userdb.credentials import CredentialProofingMethod, FidoCredential, Password from eduid.userdb.credentials.external import BankIDCredential, SwedenConnectCredential from eduid.userdb.element import ElementKey from eduid.userdb.idp import IdPUser -from eduid.common.models.saml2 import EduidAuthnContextClass from eduid.webapp.common.session.namespaces import OnetimeCredential, OnetimeCredType from eduid.webapp.idp.app import current_idp_app from eduid.webapp.idp.app import current_idp_app as current_app diff --git a/src/eduid/webapp/idp/assurance_data.py b/src/eduid/webapp/idp/assurance_data.py index 407d9e558..85e1e0d06 100644 --- a/src/eduid/webapp/idp/assurance_data.py +++ b/src/eduid/webapp/idp/assurance_data.py @@ -8,8 +8,8 @@ from pydantic import BaseModel -from eduid.userdb.element import ElementKey from eduid.common.models.saml2 import EduidAuthnContextClass +from eduid.userdb.element import ElementKey class SwamidAssurance(str, Enum): diff --git a/src/eduid/webapp/idp/other_device/db.py b/src/eduid/webapp/idp/other_device/db.py index a2bbcee71..1f5a7b8f2 100644 --- a/src/eduid/webapp/idp/other_device/db.py +++ b/src/eduid/webapp/idp/other_device/db.py @@ -13,10 +13,10 @@ from eduid.common.misc.encoders import EduidJSONEncoder from eduid.common.misc.timeutil import utc_now +from eduid.common.models.saml2 import EduidAuthnContextClass from eduid.userdb import User from eduid.userdb.db import BaseDB from eduid.webapp.common.api.utils import make_short_code -from eduid.common.models.saml2 import EduidAuthnContextClass from eduid.webapp.idp.assurance_data import UsedCredential from eduid.webapp.idp.idp_saml import ServiceInfo from eduid.webapp.idp.mischttp import get_user_agent From f0fdbdc776e46473aeae86609440206b76fac800 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Fri, 28 Jun 2024 11:29:51 +0200 Subject: [PATCH 09/11] settings -> preferences force_mfa -> always_use_security_key --- src/eduid/webapp/idp/mfa_action.py | 2 +- src/eduid/webapp/idp/tests/test_api.py | 4 ++-- src/eduid/webapp/idp/tests/test_login.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/eduid/webapp/idp/mfa_action.py b/src/eduid/webapp/idp/mfa_action.py index 9fb821837..a806209d4 100644 --- a/src/eduid/webapp/idp/mfa_action.py +++ b/src/eduid/webapp/idp/mfa_action.py @@ -19,7 +19,7 @@ def need_security_key(user: IdPUser, ticket: LoginContext) -> bool: logger.debug("User has no FIDO credentials, no extra requirement for MFA this session imposed") return False - if user.settings.force_mfa is False: + if user.preferences.always_use_security_key is False: logger.debug("User has not forced MFA, no extra requirement for MFA this session imposed") return False diff --git a/src/eduid/webapp/idp/tests/test_api.py b/src/eduid/webapp/idp/tests/test_api.py index e56206a1a..068cf67ce 100644 --- a/src/eduid/webapp/idp/tests/test_api.py +++ b/src/eduid/webapp/idp/tests/test_api.py @@ -416,7 +416,7 @@ def add_test_user_security_key( is_verified: bool = False, mfa_approved: bool = False, credential: Optional[FidoCredential] = None, - force_mfa_user_setting: bool = True, + always_use_security_key_user_preference: bool = True, ): if user is None: user = self.test_user @@ -436,7 +436,7 @@ def add_test_user_security_key( mfa_approved=mfa_approved, ) user.credentials.add(credential) - user.settings.force_mfa = force_mfa_user_setting + user.preferences.always_use_security_key = always_use_security_key_user_preference self.request_user_sync(user) def add_test_user_external_mfa_cred( diff --git a/src/eduid/webapp/idp/tests/test_login.py b/src/eduid/webapp/idp/tests/test_login.py index e824e3a19..ad2eec608 100644 --- a/src/eduid/webapp/idp/tests/test_login.py +++ b/src/eduid/webapp/idp/tests/test_login.py @@ -116,7 +116,7 @@ def test_login_no_mandatory_mfa(self) -> None: self.add_test_user_tou() # add security key to user - self.add_test_user_security_key(force_mfa_user_setting=False) + self.add_test_user_security_key(always_use_security_key_user_preference=False) # Patch the VCCSClient, so we do not need a vccs server with patch.object(VCCSClient, "authenticate") as mock_vccs: @@ -143,7 +143,7 @@ def test_login_no_mandatory_mfa_with_mfa_accr(self) -> None: self.add_test_user_tou() # add security key to user - self.add_test_user_security_key(force_mfa_user_setting=False) + self.add_test_user_security_key(always_use_security_key_user_preference=False) # Patch the VCCSClient, so we do not need a vccs server with patch.object(VCCSClient, "authenticate") as mock_vccs: From ba8c7ab4a772006fac9518fe6df326fb8d034e42 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Fri, 9 Aug 2024 15:13:12 +0200 Subject: [PATCH 10/11] mod_data already in data at this point --- src/eduid/webapp/personal_data/tests/test_app.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/eduid/webapp/personal_data/tests/test_app.py b/src/eduid/webapp/personal_data/tests/test_app.py index e94cdff31..c110f1232 100644 --- a/src/eduid/webapp/personal_data/tests/test_app.py +++ b/src/eduid/webapp/personal_data/tests/test_app.py @@ -124,8 +124,6 @@ def _post_preferences(self, mock_request_user_sync: Any, mod_data: Optional[dict with client.session_transaction() as sess: if "csrf_token" not in data: data["csrf_token"] = sess.get_csrf_token() - if mod_data: - data.update(mod_data) return client.post("/preferences", json=data) def _get_user_identities(self, eppn: Optional[str] = None): From 5667250c7d6f7c40f8e865db5bfce32d7b841649 Mon Sep 17 00:00:00 2001 From: Johan Lundberg Date: Mon, 12 Aug 2024 09:05:09 +0200 Subject: [PATCH 11/11] fixes after review --- src/eduid/webapp/idp/tests/test_SSO.py | 72 +++++++-------- src/eduid/webapp/idp/tests/test_api.py | 3 - src/eduid/webapp/idp/tests/test_login.py | 108 ++++++++++++++--------- 3 files changed, 104 insertions(+), 79 deletions(-) diff --git a/src/eduid/webapp/idp/tests/test_SSO.py b/src/eduid/webapp/idp/tests/test_SSO.py index 98ebf3da3..7da0e227e 100644 --- a/src/eduid/webapp/idp/tests/test_SSO.py +++ b/src/eduid/webapp/idp/tests/test_SSO.py @@ -274,7 +274,7 @@ def _check_login_response_authn( # ------------------------------------------------------------------------ - def test__get_login_response_1(self): + def test_get_login_response_1(self): """ Test login with password and SWAMID AL3 U2F, request REFEDS MFA. @@ -295,7 +295,7 @@ def test__get_login_response_1(self): assurance_profile=self.app.conf.swamid_assurance_profile_3, ) - def test__get_login_response_2(self): + def test_get_login_response_2(self): """ Test login with password and U2F, request REFEDS MFA. @@ -316,7 +316,7 @@ def test__get_login_response_2(self): assurance_profile=self.app.conf.swamid_assurance_profile_2, ) - def test__get_login_response_external_multifactor(self): + def test_get_login_response_external_multifactor(self): """ Test login with password and external MFA, request REFEDS MFA. @@ -340,7 +340,7 @@ def test__get_login_response_external_multifactor(self): assurance_profile=self.app.conf.swamid_assurance_profile_3, ) - def test__get_login_response_3(self): + def test_get_login_response_3(self): """ Test login with password and U2F, request REFEDS SFA. @@ -357,7 +357,7 @@ def test__get_login_response_3(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_response_4(self): + def test_get_login_response_4(self): """ Test login with password, request REFEDS SFA. @@ -374,7 +374,7 @@ def test__get_login_response_4(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_response_UNSPECIFIED2(self): + def test_get_login_response_UNSPECIFIED2(self): """ Test login with U2F, request REFEDS SFA. @@ -391,7 +391,7 @@ def test__get_login_response_UNSPECIFIED2(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_response_5(self): + def test_get_login_response_5(self): """ Test login with password and U2F, request FIDO U2F. @@ -408,7 +408,7 @@ def test__get_login_response_5(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_response_6(self): + def test_get_login_response_6(self): """ Test login with password and U2F, request plain password-protected-transport. @@ -425,7 +425,7 @@ def test__get_login_response_6(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_response_7(self): + def test_get_login_response_7(self): """ Test login with password, request plain password-protected-transport. @@ -442,7 +442,7 @@ def test__get_login_response_7(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_response_8(self): + def test_get_login_response_8(self): """ Test login with mfa, request unknown context class. @@ -456,7 +456,7 @@ def test__get_login_response_8(self): authn_result=out, message=IdPMsg.assurance_failure, expect_success=False, expect_error=True ) - def test__get_login_response_9(self): + def test_get_login_response_9(self): """ Test login with password, request unknown context class. @@ -470,7 +470,7 @@ def test__get_login_response_9(self): authn_result=out, message=IdPMsg.assurance_failure, expect_success=False, expect_error=True ) - def test__get_login_response_10(self): + def test_get_login_response_10(self): """ Test login with password, request no authn context class. @@ -487,7 +487,7 @@ def test__get_login_response_10(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_response_11(self): + def test_get_login_response_11(self): """ Test login with mfa, request no authn context class. @@ -504,7 +504,7 @@ def test__get_login_response_11(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_response_assurance_AL1(self): + def test_get_login_response_assurance_AL1(self): """ Make sure eduPersonAssurace is SWAMID AL1 with no verified nin. """ @@ -519,7 +519,7 @@ def test__get_login_response_assurance_AL1(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_response_assurance_AL2(self): + def test_get_login_response_assurance_AL2(self): """ Make sure eduPersonAssurace is SWAMID AL2 with a verified nin. """ @@ -536,7 +536,7 @@ def test__get_login_response_assurance_AL2(self): assurance_profile=self.app.conf.swamid_assurance_profile_2, ) - def test__get_login_eduid_mfa_fido_al1(self): + def test_get_login_eduid_mfa_fido_al1(self): """ Test login with password and fido for not verified user, request EDUID_MFA. @@ -553,7 +553,7 @@ def test__get_login_eduid_mfa_fido_al1(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_refeds_mfa_fido_al1(self): + def test_get_login_refeds_mfa_fido_al1(self): """ Test login with password and fido for not verified user, request REFEDS_MFA. @@ -570,7 +570,7 @@ def test__get_login_refeds_mfa_fido_al1(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_eduid_mfa_fido_al2(self): + def test_get_login_eduid_mfa_fido_al2(self): """ Test login with password and fido for verified user, request EDUID_MFA. @@ -589,11 +589,11 @@ def test__get_login_eduid_mfa_fido_al2(self): assurance_profile=self.app.conf.swamid_assurance_profile_2, ) - def test__get_login_refeds_mfa_fido_al2(self): + def test_get_login_refeds_mfa_fido_al2(self): """ Test login with password and fido for verified user, request EDUID_MFA. - Expect the response Authn to be EDUID_MFA, eduPersonAssurance AL1,Al2 + Expect the response Authn to be REFEDS_MFA, eduPersonAssurance AL1,Al2 """ user = self.get_user_set_nins(self.test_user.eppn, ["190101011234"]) out = self._get_login_response_authn( @@ -608,7 +608,7 @@ def test__get_login_refeds_mfa_fido_al2(self): assurance_profile=self.app.conf.swamid_assurance_profile_2, ) - def test__get_login_eduid_mfa_fido_swamid_al2(self): + def test_get_login_eduid_mfa_fido_swamid_al2(self): """ Test login with password and fido_swamid_al2 for verified user, request EDUID_MFA. @@ -628,7 +628,7 @@ def test__get_login_eduid_mfa_fido_swamid_al2(self): assurance_profile=self.app.conf.swamid_assurance_profile_2, ) - def test__get_login_eduid_mfa_fido_swamid_al3(self): + def test_get_login_eduid_mfa_fido_swamid_al3(self): """ Test login with password and fido_swamid_al3 for verified user, request EDUID_MFA. @@ -649,7 +649,7 @@ def test__get_login_eduid_mfa_fido_swamid_al3(self): assurance_profile=self.app.conf.swamid_assurance_profile_3, ) - def test__get_login_refeds_mfa_fido_swamid_al3(self): + def test_get_login_refeds_mfa_fido_swamid_al3(self): """ Test login with password and fido_swamid_al3 for verified user, request REFEDS_MFA. @@ -670,7 +670,7 @@ def test__get_login_refeds_mfa_fido_swamid_al3(self): assurance_profile=self.app.conf.swamid_assurance_profile_3, ) - def test__get_login_eduid_mfa_external_mfa_al3(self): + def test_get_login_eduid_mfa_external_mfa_al3(self): """ Test login with password and external mfa for verified user, request EDUID_MFA. @@ -694,11 +694,11 @@ def test__get_login_eduid_mfa_external_mfa_al3(self): assurance_profile=self.app.conf.swamid_assurance_profile_3, ) - def test__get_login_refeds_mfa_external_mfa(self): + def test_get_login_refeds_mfa_external_mfa(self): """ Test login with password and external mfa for verified user, request REFEDS_MFA. - Expect the response Authn to be EDUID_MFA. + Expect the response Authn to be REFEDS_MFA. """ user = self.get_user_set_nins(self.test_user.eppn, ["190101011234"]) external_mfa = ExternalMfaData( @@ -718,7 +718,7 @@ def test__get_login_refeds_mfa_external_mfa(self): assurance_profile=self.app.conf.swamid_assurance_profile_3, ) - def test__get_login_refeds_mfa_fido_al1_with_al3_mfa(self): + def test_get_login_refeds_mfa_fido_al1_with_al3_mfa(self): """ Test login with password and fido for not verified user, request REFEDS_MFA. @@ -738,7 +738,7 @@ def test__get_login_refeds_mfa_fido_al1_with_al3_mfa(self): assurance_profile=self.app.conf.swamid_assurance_profile_1, ) - def test__get_login_response_eduid_mfa_no_multifactor(self): + def test_get_login_response_eduid_mfa_no_multifactor(self): """ Test login with password, request EDUID_MFA. @@ -747,7 +747,7 @@ def test__get_login_response_eduid_mfa_no_multifactor(self): out = self._get_login_response_authn(req_class_ref=EduidAuthnContextClass.EDUID_MFA, credentials=["pw"]) self._check_login_response_authn(authn_result=out, message=IdPMsg.mfa_required, expect_success=False) - def test__get_login_response_refeds_mfa_no_multifactor(self): + def test_get_login_response_refeds_mfa_no_multifactor(self): """ Test login with password, request EDUID_MFA. @@ -756,7 +756,7 @@ def test__get_login_response_refeds_mfa_no_multifactor(self): out = self._get_login_response_authn(req_class_ref=EduidAuthnContextClass.REFEDS_MFA, credentials=["pw"]) self._check_login_response_authn(authn_result=out, message=IdPMsg.mfa_required, expect_success=False) - def test__get_login_digg_loa2_fido_mfa(self): + def test_get_login_digg_loa2_fido_mfa(self): """ Test login with password and fido mfa for verified user, request DIGG_LOA2. @@ -777,7 +777,7 @@ def test__get_login_digg_loa2_fido_mfa(self): assurance_profile=self.app.conf.swamid_assurance_profile_3, ) - def test__get_login_digg_loa2_fido_mfa_no_identity_proofing_method(self): + def test_get_login_digg_loa2_fido_mfa_no_identity_proofing_method(self): """ Test login with password and external mfa for verified user, request DIGG_LOA2. @@ -817,7 +817,7 @@ def test__get_login_digg_loa2_fido_mfa_no_identity_proofing_method(self): expect_error=True, ) - def test__get_login_digg_loa2_external_mfa(self): + def test_get_login_digg_loa2_external_mfa(self): """ Test login with password and external mfa for verified user, request DIGG_LOA2. @@ -843,7 +843,7 @@ def test__get_login_digg_loa2_external_mfa(self): assurance_profile=self.app.conf.swamid_assurance_profile_3, ) - def test__get_login_digg_loa2_identity_proofing_method_not_allowed(self): + def test_get_login_digg_loa2_identity_proofing_method_not_allowed(self): """ Test login with password and external mfa for verified user, request DIGG_LOA2. @@ -869,7 +869,7 @@ def test__get_login_digg_loa2_identity_proofing_method_not_allowed(self): expect_error=True, ) - def test__get_login_digg_loa2_mfa_proofing_method_not_allowed(self): + def test_get_login_digg_loa2_mfa_proofing_method_not_allowed(self): """ Test login with password and external mfa for verified user, request DIGG_LOA2. @@ -890,7 +890,7 @@ def test__get_login_digg_loa2_mfa_proofing_method_not_allowed(self): expect_error=True, ) - def test__forceauthn_request(self): + def test_forceauthn_request(self): """ForceAuthn can apparently be either 'true' or '1'. https://lists.oasis-open.org/archives/security-services/201402/msg00019.html @@ -932,7 +932,7 @@ def test__forceauthn_request(self): assert x.force_authn == expected - def test__service_info(self): + def test_service_info(self): with self.app.test_request_context(): ticket = self._make_login_ticket(EduidAuthnContextClass.PASSWORD_PT) diff --git a/src/eduid/webapp/idp/tests/test_api.py b/src/eduid/webapp/idp/tests/test_api.py index 068cf67ce..c5f9fb471 100644 --- a/src/eduid/webapp/idp/tests/test_api.py +++ b/src/eduid/webapp/idp/tests/test_api.py @@ -468,15 +468,12 @@ def get_attributes(self, result, saml2_client: Optional[Saml2Client] = None): return attributes def _assert_dict_contains(self, actual: dict[str, Any], expected: dict[str, Any]): - # try: for key, value in expected.items(): assert key in actual, f"expected {key} not in {actual}" if isinstance(value, dict): self._assert_dict_contains(actual[key], value) else: assert actual[key] == value, f"expected {key} value: {actual[key]} != {value} in {actual}" - # except AssertionError as e: - # raise AssertionError(f"{e}\n\nactual: {actual}") def _check_login_result( self, diff --git a/src/eduid/webapp/idp/tests/test_login.py b/src/eduid/webapp/idp/tests/test_login.py index ad2eec608..c7f33fcbd 100644 --- a/src/eduid/webapp/idp/tests/test_login.py +++ b/src/eduid/webapp/idp/tests/test_login.py @@ -31,15 +31,26 @@ class IdPTestLoginAPI(IdPAPITests): def test_login_start(self) -> None: result = self._try_login(test_user=TestUser(eppn=None, password=None)) - assert result.visit_order == [IdPAction.PWAUTH] - assert result.sso_cookie_val is None + + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH], + sso_cookie_val=None, + ) def test_login_pwauth_wrong_password(self) -> None: result = self._try_login() - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.PWAUTH] - assert result.sso_cookie_val is None - assert result.pwauth_result is not None - assert result.pwauth_result.payload["message"] == IdPMsg.wrong_credentials.value + + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.PWAUTH], + sso_cookie_val=None, + pwauth_result=PwAuthResult( + payload={ + "message": IdPMsg.wrong_credentials.value, + } + ), + ) def test_login_pwauth_right_password(self) -> None: # pre-accept ToU for this test @@ -50,12 +61,17 @@ def test_login_pwauth_right_password(self) -> None: mock_vccs.return_value = True result = self._try_login() - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.FINISHED] - assert result.sso_cookie_val is not None - assert result.finished_result is not None - assert result.finished_result.payload["message"] == IdPMsg.finished.value - assert result.finished_result.payload["target"] == "https://sp.example.edu/saml2/acs/" - assert result.finished_result.payload["parameters"]["RelayState"] == self.relay_state + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.FINISHED], + finish_result=FinishedResultAPI( + payload={ + "message": IdPMsg.finished.value, + "target": "https://sp.example.edu/saml2/acs/", + "parameters": {"RelayState": self.relay_state}, + } + ), + ) attributes = self.get_attributes(result) @@ -71,15 +87,19 @@ def test_login_pwauth_right_password_and_tou_acceptance(self) -> None: mock_vccs.return_value = True result = self._try_login() - assert result.visit_order == [IdPAction.PWAUTH, IdPAction.TOU, IdPAction.FINISHED] - assert result.sso_cookie_val is not None - assert result.finished_result is not None - assert result.finished_result.payload["message"] == IdPMsg.finished.value - assert result.finished_result.payload["target"] == "https://sp.example.edu/saml2/acs/" - assert result.finished_result.payload["parameters"]["RelayState"] == self.relay_state + self._check_login_result( + result=result, + visit_order=[IdPAction.PWAUTH, IdPAction.TOU, IdPAction.FINISHED], + finish_result=FinishedResultAPI( + payload={ + "message": IdPMsg.finished.value, + "target": "https://sp.example.edu/saml2/acs/", + "parameters": {"RelayState": self.relay_state}, + } + ), + ) attributes = self.get_attributes(result) - assert "eduPersonPrincipalName" in attributes assert attributes["eduPersonPrincipalName"] == [f"hubba-bubba@{self.app.conf.default_eppn_scope}"] @@ -95,19 +115,23 @@ def test_login_mfaauth(self) -> None: mock_vccs.return_value = True result = self._try_login() - assert result.visit_order == [ - IdPAction.PWAUTH, - IdPAction.MFA, - IdPAction.FINISHED, - ], f"NOT expected visit order {result.visit_order}" - assert result.sso_cookie_val is not None - assert result.finished_result is not None - assert result.finished_result.payload["message"] == IdPMsg.finished.value - assert result.finished_result.payload["target"] == "https://sp.example.edu/saml2/acs/" - assert result.finished_result.payload["parameters"]["RelayState"] == self.relay_state + self._check_login_result( + result=result, + visit_order=[ + IdPAction.PWAUTH, + IdPAction.MFA, + IdPAction.FINISHED, + ], + finish_result=FinishedResultAPI( + payload={ + "message": IdPMsg.finished.value, + "target": "https://sp.example.edu/saml2/acs/", + "parameters": {"RelayState": self.relay_state}, + } + ), + ) attributes = self.get_attributes(result) - assert "eduPersonPrincipalName" in attributes assert attributes["eduPersonPrincipalName"] == [f"hubba-bubba@{self.app.conf.default_eppn_scope}"] @@ -123,18 +147,22 @@ def test_login_no_mandatory_mfa(self) -> None: mock_vccs.return_value = True result = self._try_login() - assert result.visit_order == [ - IdPAction.PWAUTH, - IdPAction.FINISHED, - ], f"NOT expected visit order {result.visit_order}" - assert result.sso_cookie_val is not None - assert result.finished_result is not None - assert result.finished_result.payload["message"] == IdPMsg.finished.value - assert result.finished_result.payload["target"] == "https://sp.example.edu/saml2/acs/" - assert result.finished_result.payload["parameters"]["RelayState"] == self.relay_state + self._check_login_result( + result=result, + visit_order=[ + IdPAction.PWAUTH, + IdPAction.FINISHED, + ], + finish_result=FinishedResultAPI( + payload={ + "message": IdPMsg.finished.value, + "target": "https://sp.example.edu/saml2/acs/", + "parameters": {"RelayState": self.relay_state}, + } + ), + ) attributes = self.get_attributes(result) - assert "eduPersonPrincipalName" in attributes assert attributes["eduPersonPrincipalName"] == [f"hubba-bubba@{self.app.conf.default_eppn_scope}"]