Merge pull request #676 from SUNET/lundberg_authn_fixes

security zone authn fixes

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}",

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,

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)

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:

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(

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")

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
 

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)

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:

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(

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:

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
 

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:

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"])

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
 

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
 

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