diff --git a/src/eduid/common/config/base.py b/src/eduid/common/config/base.py index cd24abc8c..906148af8 100644 --- a/src/eduid/common/config/base.py +++ b/src/eduid/common/config/base.py @@ -382,6 +382,7 @@ class FrontendActionMixin(BaseModel): frontend_action_authn_parameters: dict[FrontendAction, AuthnParameters] = Field( default={ FrontendAction.ADD_SECURITY_KEY_AUTHN: AuthnParameters( + force_authn=True, high_security=True, allow_login_auth=True, finish_url="https://eduid.se/profile/ext-return/{app_name}/{authn_id}", @@ -412,6 +413,7 @@ class FrontendActionMixin(BaseModel): finish_url="https://eduid.se/profile/", ), FrontendAction.REMOVE_SECURITY_KEY_AUTHN: AuthnParameters( + force_authn=True, force_mfa=True, allow_login_auth=True, finish_url="https://eduid.se/profile/ext-return/{app_name}/{authn_id}", diff --git a/src/eduid/webapp/authn/views.py b/src/eduid/webapp/authn/views.py index 80b6bb450..6ed121186 100644 --- a/src/eduid/webapp/authn/views.py +++ b/src/eduid/webapp/authn/views.py @@ -115,6 +115,7 @@ def authenticate( try: action = FrontendAction(frontend_action) authn_params = current_app.conf.frontend_action_authn_parameters[action] + current_app.logger.debug(f"Authn parameters for frontend action {action}: {authn_params}") except (ValueError, KeyError): current_app.logger.exception(f"Frontend action {frontend_action} not supported") return error_response(message=AuthnMsg.frontend_action_not_supported) @@ -131,6 +132,9 @@ def authenticate( ) if authn_params.force_mfa or _request_mfa: + current_app.logger.debug( + f"Forcing MFA authentication. force_mfa: {authn_params.force_mfa}, request_mfa: {_request_mfa}" + ) req_authn_ctx = [REFEDS_MFA] sp_authn = SP_AuthnRequest( @@ -231,6 +235,7 @@ def _authn(sp_authn: SP_AuthnRequest, idp: str, authn_params: AuthnParameters) - authn_id=sp_authn.authn_id, selected_idp=idp, force_authn=authn_params.force_authn, + req_authn_ctx=sp_authn.req_authn_ctx, sign_alg=current_app.conf.authn_sign_alg, digest_alg=current_app.conf.authn_digest_alg, subject=subject, diff --git a/src/eduid/webapp/bankid/acs_actions.py b/src/eduid/webapp/bankid/acs_actions.py index c8aedb2c8..44c6f431f 100644 --- a/src/eduid/webapp/bankid/acs_actions.py +++ b/src/eduid/webapp/bankid/acs_actions.py @@ -107,7 +107,7 @@ def verify_credential_action(user: User, args: ACSArgs) -> ACSResult: return ACSResult(message=BankIDMsg.credential_not_found) # Check (again) if token was used to authenticate this session and that the auth is not stale. - _need_reauthn = check_reauthn(frontend_action=args.authn_req.frontend_action, credential_used=credential) + _need_reauthn = check_reauthn(frontend_action=args.authn_req.frontend_action, user=user, credential_used=credential) if _need_reauthn: current_app.logger.error(f"User needs to authenticate: {_need_reauthn}") return ACSResult(message=AuthnStatusMsg.must_authenticate) diff --git a/src/eduid/webapp/bankid/helpers.py b/src/eduid/webapp/bankid/helpers.py index d70098515..e0fec615e 100644 --- a/src/eduid/webapp/bankid/helpers.py +++ b/src/eduid/webapp/bankid/helpers.py @@ -7,6 +7,7 @@ from saml2.typing import SAMLHttpArgs from eduid.common.config.base import FrontendAction +from eduid.userdb import User from eduid.userdb.credentials import Credential from eduid.userdb.credentials.external import TrustFramework from eduid.webapp.bankid.app import current_bankid_app as current_app @@ -96,12 +97,12 @@ def create_authn_info( def check_reauthn( - frontend_action: FrontendAction, credential_used: Optional[Credential] = None + frontend_action: FrontendAction, user: User, credential_used: Optional[Credential] = None ) -> Optional[AuthnActionStatus]: """Check if a re-authentication has been performed recently enough for this action""" authn_status = validate_authn_for_action( - config=current_app.conf, frontend_action=frontend_action, credential_used=credential_used + config=current_app.conf, frontend_action=frontend_action, credential_used=credential_used, user=user ) current_app.logger.debug(f"check_reauthn called with authn status {authn_status}") if authn_status != AuthnActionStatus.OK: diff --git a/src/eduid/webapp/bankid/views.py b/src/eduid/webapp/bankid/views.py index e845bbb45..6a43de84f 100644 --- a/src/eduid/webapp/bankid/views.py +++ b/src/eduid/webapp/bankid/views.py @@ -95,7 +95,7 @@ def verify_credential( current_app.logger.error(f"Can't find credential with id: {credential_id}") return error_response(message=BankIDMsg.credential_not_found) - _need_reauthn = check_reauthn(frontend_action=_frontend_action, credential_used=credential) + _need_reauthn = check_reauthn(frontend_action=_frontend_action, user=user, credential_used=credential) if _need_reauthn: current_app.logger.debug(f"Need re-authentication for credential: {credential_id}") return need_authentication_response( diff --git a/src/eduid/webapp/common/authn/eduid_saml2.py b/src/eduid/webapp/common/authn/eduid_saml2.py index 8ffde65f5..a22b555c0 100644 --- a/src/eduid/webapp/common/authn/eduid_saml2.py +++ b/src/eduid/webapp/common/authn/eduid_saml2.py @@ -61,6 +61,7 @@ def get_authn_request( authn_id: AuthnRequestRef, selected_idp: Optional[str], force_authn: bool = False, + req_authn_ctx: Optional[list] = None, sign_alg: Optional[str] = None, digest_alg: Optional[str] = None, subject: Optional[Subject] = None, @@ -69,6 +70,12 @@ def get_authn_request( client = Saml2Client(saml2_config) + # authn context class + kwargs: dict[str, Any] = {} + if req_authn_ctx is not None: + logger.debug(f"Requesting AuthnContext {req_authn_ctx}") + kwargs["requested_authn_context"] = {"authn_context_class_ref": req_authn_ctx, "comparison": "exact"} + try: session_id: str info: Mapping[str, Any] @@ -80,6 +87,7 @@ def get_authn_request( digest_alg=digest_alg, subject=subject, force_authn=str(force_authn).lower(), + **kwargs, ) except TypeError: logger.error("Unable to know which IdP to use") diff --git a/src/eduid/webapp/common/authn/utils.py b/src/eduid/webapp/common/authn/utils.py index 496329da0..00a0dfff8 100644 --- a/src/eduid/webapp/common/authn/utils.py +++ b/src/eduid/webapp/common/authn/utils.py @@ -12,7 +12,8 @@ from eduid.common.config.exceptions import BadConfiguration from eduid.common.misc.timeutil import utc_now from eduid.common.utils import urlappend -from eduid.userdb.credentials import Credential +from eduid.userdb import User +from eduid.userdb.credentials import Credential, FidoCredential from eduid.webapp.common.api.schemas.authn_status import AuthnActionStatus from eduid.webapp.common.authn.session_info import SessionInfo from eduid.webapp.common.session import session @@ -128,6 +129,7 @@ def get_authn_for_action( def validate_authn_for_action( config: FrontendActionMixin, frontend_action: FrontendAction, + user: User, credential_used: Optional[Credential] = None, ) -> AuthnActionStatus: """ @@ -158,9 +160,16 @@ def validate_authn_for_action( logger.info(f"Expected accr: {authn.req_authn_ctx} got: {authn.asserted_authn_ctx}") return AuthnActionStatus.WRONG_ACCR + # optimistic check for MFA aka "high security" + if authn_params.high_security and len(authn.credentials_used) < 2: + if len(user.credentials.filter(FidoCredential)) >= 1: + logger.info("Authentication (high_security) requires MFA") + logger.info(f"Expected at least 2 credentials got: {len(authn.credentials_used)}") + return AuthnActionStatus.NO_MFA + # specific check for MFA to be able to use login actions if authn_params.force_mfa and len(authn.credentials_used) < 2: - logger.info("Authentication requires MFA") + logger.info("Authentication (force_mfa) requires MFA") logger.info(f"Expected at least 2 credentials got: {len(authn.credentials_used)}") return AuthnActionStatus.NO_MFA diff --git a/src/eduid/webapp/eidas/acs_actions.py b/src/eduid/webapp/eidas/acs_actions.py index 5288ec3a3..d825f5639 100644 --- a/src/eduid/webapp/eidas/acs_actions.py +++ b/src/eduid/webapp/eidas/acs_actions.py @@ -111,7 +111,7 @@ def verify_credential_action(user: User, args: ACSArgs) -> ACSResult: return ACSResult(message=EidasMsg.credential_not_found) # Check (again) if token was used to authenticate this session and that the auth is not stale. - _need_reauthn = check_reauthn(frontend_action=args.authn_req.frontend_action, credential_used=credential) + _need_reauthn = check_reauthn(frontend_action=args.authn_req.frontend_action, user=user, credential_used=credential) if _need_reauthn: current_app.logger.error(f"User needs to authenticate: {_need_reauthn}") return ACSResult(message=AuthnStatusMsg.must_authenticate) diff --git a/src/eduid/webapp/eidas/helpers.py b/src/eduid/webapp/eidas/helpers.py index cb123886a..c74bb62c4 100644 --- a/src/eduid/webapp/eidas/helpers.py +++ b/src/eduid/webapp/eidas/helpers.py @@ -8,6 +8,7 @@ from saml2.typing import SAMLHttpArgs from eduid.common.config.base import FrontendAction +from eduid.userdb import User from eduid.userdb.credentials import Credential from eduid.userdb.credentials.external import TrustFramework from eduid.webapp.common.api.messages import TranslatableMsg @@ -122,12 +123,12 @@ class CredentialVerifyResult: def check_reauthn( - frontend_action: FrontendAction, credential_used: Optional[Credential] = None + frontend_action: FrontendAction, user: User, credential_used: Optional[Credential] = None ) -> Optional[AuthnActionStatus]: """Check if a re-authentication has been performed recently enough for this action""" authn_status = validate_authn_for_action( - config=current_app.conf, frontend_action=frontend_action, credential_used=credential_used + config=current_app.conf, frontend_action=frontend_action, credential_used=credential_used, user=user ) current_app.logger.debug(f"check_reauthn called with authn status {authn_status}") if authn_status != AuthnActionStatus.OK: diff --git a/src/eduid/webapp/eidas/views.py b/src/eduid/webapp/eidas/views.py index 96d2fa701..dadecf6fb 100644 --- a/src/eduid/webapp/eidas/views.py +++ b/src/eduid/webapp/eidas/views.py @@ -95,7 +95,7 @@ def verify_credential( current_app.logger.error(f"Can't find credential with id: {credential_id}") return error_response(message=EidasMsg.credential_not_found) - _need_reauthn = check_reauthn(frontend_action=_frontend_action, credential_used=credential) + _need_reauthn = check_reauthn(frontend_action=_frontend_action, user=user, credential_used=credential) if _need_reauthn: current_app.logger.debug(f"Need re-authentication for credential: {credential_id}") return need_authentication_response( diff --git a/src/eduid/webapp/personal_data/helpers.py b/src/eduid/webapp/personal_data/helpers.py index 390c38cd8..b684b1caf 100644 --- a/src/eduid/webapp/personal_data/helpers.py +++ b/src/eduid/webapp/personal_data/helpers.py @@ -4,6 +4,7 @@ from typing import Optional from eduid.common.config.base import FrontendAction +from eduid.userdb import User from eduid.webapp.common.api.messages import FluxData, TranslatableMsg, need_authentication_response from eduid.webapp.common.api.schemas.authn_status import AuthnActionStatus from eduid.webapp.common.authn.utils import validate_authn_for_action @@ -71,10 +72,10 @@ def is_valid_chosen_given_name(given_name: Optional[str] = None, chosen_given_na return False -def check_reauthn(frontend_action: FrontendAction) -> Optional[FluxData]: +def check_reauthn(frontend_action: FrontendAction, user: User) -> Optional[FluxData]: """Check if a re-authentication has been performed recently enough for this action""" - authn_status = validate_authn_for_action(config=current_app.conf, frontend_action=frontend_action) + authn_status = validate_authn_for_action(config=current_app.conf, frontend_action=frontend_action, user=user) current_app.logger.debug(f"check_reauthn called with authn status {authn_status}") if authn_status != AuthnActionStatus.OK: if authn_status == AuthnActionStatus.STALE: diff --git a/src/eduid/webapp/personal_data/views.py b/src/eduid/webapp/personal_data/views.py index 96ed13b39..a63dbc205 100644 --- a/src/eduid/webapp/personal_data/views.py +++ b/src/eduid/webapp/personal_data/views.py @@ -103,7 +103,7 @@ def set_user_preferences(user: User, always_use_security_key: bool) -> FluxData: # and have a separate endpoint for each group and FrontendAction. frontend_action = FrontendAction.CHANGE_SECURITY_PREFERENCES_AUTHN - _need_reauthn = check_reauthn(frontend_action=frontend_action) + _need_reauthn = check_reauthn(frontend_action=frontend_action, user=user) if _need_reauthn: return _need_reauthn diff --git a/src/eduid/webapp/security/helpers.py b/src/eduid/webapp/security/helpers.py index 723b1cf98..5080f5887 100644 --- a/src/eduid/webapp/security/helpers.py +++ b/src/eduid/webapp/security/helpers.py @@ -191,10 +191,10 @@ def send_termination_mail(user): current_app.logger.info(f"Sent termination mail to user {user} to address {email}.") -def check_reauthn(frontend_action: FrontendAction) -> Optional[FluxData]: +def check_reauthn(frontend_action: FrontendAction, user: User) -> Optional[FluxData]: """Check if a re-authentication has been performed recently enough for this action""" - authn_status = validate_authn_for_action(config=current_app.conf, frontend_action=frontend_action) + authn_status = validate_authn_for_action(config=current_app.conf, frontend_action=frontend_action, user=user) current_app.logger.debug(f"check_reauthn called with authn status {authn_status}") if authn_status != AuthnActionStatus.OK: if authn_status == AuthnActionStatus.STALE: diff --git a/src/eduid/webapp/security/tests/test_webauthn.py b/src/eduid/webapp/security/tests/test_webauthn.py index 32defd81e..ef262c17f 100644 --- a/src/eduid/webapp/security/tests/test_webauthn.py +++ b/src/eduid/webapp/security/tests/test_webauthn.py @@ -7,7 +7,7 @@ from werkzeug.http import dump_cookie from eduid.common.config.base import EduidEnvironment, FrontendAction -from eduid.userdb.credentials import U2F, Webauthn +from eduid.userdb.credentials import U2F, FidoCredential, Webauthn from eduid.webapp.common.api.testing import EduidAPITestCase from eduid.webapp.common.session import EduidSession from eduid.webapp.common.session.namespaces import WebauthnRegistration, WebauthnState @@ -96,6 +96,16 @@ class SecurityWebauthnTests(EduidAPITestCase): app: SecurityApp + def setUp(self): + super().setUp() + # remove all FidoCredentials from the test user + user = self.app.central_userdb.get_user_by_eppn(self.test_user_eppn) + assert user is not None + for credential in user.credentials: + if isinstance(credential, FidoCredential): + user.credentials.remove(credential.key) + self.app.central_userdb.save(user) + def load_app(self, config: Mapping[str, Any]) -> SecurityApp: """ Called from the parent class, so we can provide the appropriate flask @@ -198,9 +208,16 @@ def _begin_register_key( :param csrf: to control the CSRF token to send :param check_session: whether to check the registration state in the session """ + + force_mfa = False + if other is not None or existing_legacy_token: + # Fake that user used the other security key to authenticate + force_mfa = True + self.set_authn_action( eppn=self.test_user_eppn, frontend_action=FrontendAction.ADD_SECURITY_KEY_AUTHN, + force_mfa=force_mfa, ) if existing_legacy_token: @@ -250,9 +267,15 @@ def _finish_register_key( """ mock_request_user_sync.side_effect = self.request_user_sync + force_mfa = False + if existing_legacy_token: + # Fake that user used the other security key to authenticate + force_mfa = True + self.set_authn_action( eppn=self.test_user_eppn, frontend_action=FrontendAction.ADD_SECURITY_KEY_AUTHN, + force_mfa=force_mfa, ) if existing_legacy_token: @@ -639,6 +662,6 @@ def test_approved_security_keys(self): assert "entries" in payload assert len(payload["entries"]) > 0 - # test no dubles + # test no doubles unique_lowecase_entries = list(set(e.lower() for e in payload["entries"])) assert len(unique_lowecase_entries) == len(payload["entries"]) diff --git a/src/eduid/webapp/security/views/change_password.py b/src/eduid/webapp/security/views/change_password.py index cb5c901ad..955da805f 100644 --- a/src/eduid/webapp/security/views/change_password.py +++ b/src/eduid/webapp/security/views/change_password.py @@ -38,7 +38,7 @@ def get_suggested(user: User) -> FluxData: View to get a suggested password for the logged user. """ - _need_reauthn = check_reauthn(frontend_action=FrontendAction.CHANGE_PW_AUTHN) + _need_reauthn = check_reauthn(frontend_action=FrontendAction.CHANGE_PW_AUTHN, user=user) if _need_reauthn: return _need_reauthn @@ -58,7 +58,7 @@ def change_password_view(user: User, new_password: str, old_password: Optional[s """ frontend_action = FrontendAction.CHANGE_PW_AUTHN - _need_reauthn = check_reauthn(frontend_action=frontend_action) + _need_reauthn = check_reauthn(frontend_action=frontend_action, user=user) if _need_reauthn: return _need_reauthn diff --git a/src/eduid/webapp/security/views/security.py b/src/eduid/webapp/security/views/security.py index f4d6f41ce..844e2c284 100644 --- a/src/eduid/webapp/security/views/security.py +++ b/src/eduid/webapp/security/views/security.py @@ -68,7 +68,7 @@ def terminate_account(user: User): """ frontend_action = FrontendAction.TERMINATE_ACCOUNT_AUTHN - _need_reauthn = check_reauthn(frontend_action=frontend_action) + _need_reauthn = check_reauthn(frontend_action=frontend_action, user=user) if _need_reauthn: return _need_reauthn @@ -180,7 +180,7 @@ def remove_identities(user: User, identity_type: str) -> FluxData: frontend_action = FrontendAction.REMOVE_IDENTITY - _need_reauthn = check_reauthn(frontend_action=frontend_action) + _need_reauthn = check_reauthn(frontend_action=frontend_action, user=user) if _need_reauthn: return _need_reauthn diff --git a/src/eduid/webapp/security/views/webauthn.py b/src/eduid/webapp/security/views/webauthn.py index 31903b035..9ea99fc5f 100644 --- a/src/eduid/webapp/security/views/webauthn.py +++ b/src/eduid/webapp/security/views/webauthn.py @@ -89,7 +89,7 @@ def registration_begin(user: User, authenticator: str) -> FluxData: frontend_action = FrontendAction.ADD_SECURITY_KEY_AUTHN - _need_reauthn = check_reauthn(frontend_action=frontend_action) + _need_reauthn = check_reauthn(frontend_action=frontend_action, user=user) if _need_reauthn: return _need_reauthn @@ -149,7 +149,7 @@ def registration_complete( ) -> FluxData: frontend_action = FrontendAction.ADD_SECURITY_KEY_AUTHN - _need_reauthn = check_reauthn(frontend_action=frontend_action) + _need_reauthn = check_reauthn(frontend_action=frontend_action, user=user) if _need_reauthn: return _need_reauthn @@ -242,7 +242,7 @@ def remove(user: User, credential_key: str) -> FluxData: frontend_action = FrontendAction.REMOVE_SECURITY_KEY_AUTHN - _need_reauthn = check_reauthn(frontend_action=frontend_action) + _need_reauthn = check_reauthn(frontend_action=frontend_action, user=user) if _need_reauthn: return _need_reauthn