Merge pull request #725 from SUNET/lundberg_webauthn_update

switch from default deny to default allow for security keys

src/eduid/scimapi/testing.py

@@ -288,9 +288,9 @@ def _assertName(db_name: ScimApiName, response_name: dict[str, str]) -> None:
         ]
         db_name_dict = asdict(db_name)
         for first, second in name_map:
-            assert db_name_dict.get(first) == response_name.get(second), (
-                f"{first}:{db_name_dict.get(first)} != {second}:{response_name.get(second)}"
-            )
+            assert db_name_dict.get(first) == response_name.get(
+                second
+            ), f"{first}:{db_name_dict.get(first)} != {second}:{response_name.get(second)}"
 
     @staticmethod
     def _assertResponse(response: Response, status_code: int = 200) -> None:
@@ -300,6 +300,6 @@ def _assertResponse(response: Response, status_code: int = 200) -> None:
                 _detail = response.json().get("detail", "No error detail in parsed_response")
         except JSONDecodeError:
             pass
-        assert response.status_code == status_code, (
-            f"Response status was not {status_code} ({response.status_code}), {_detail}"
-        )
+        assert (
+            response.status_code == status_code
+        ), f"Response status was not {status_code} ({response.status_code}), {_detail}"

src/eduid/scimapi/tests/test_scimuser.py

@@ -204,19 +204,19 @@ def _assertUserUpdateSuccess(self, req: Mapping, response: Response, user: ScimA
         self._assertScimResponseProperties(response, resource=user, expected_schemas=expected_schemas)
 
         # Validate user update specifics
-        assert user.external_id == response.json().get("externalId"), (
-            'user.externalId != parsed_response.json().get("externalId")'
-        )
+        assert user.external_id == response.json().get(
+            "externalId"
+        ), 'user.externalId != parsed_response.json().get("externalId")'
         self._assertName(user.name, response.json().get("name"))
         _expected_emails = filter_none(normalised_data([email.to_dict() for email in user.emails]))
         _obtained_emails = filter_none(normalised_data(response.json().get("emails", [])))
         assert _obtained_emails == _expected_emails, 'parsed_response.json().get("email") != user.emails'
         _expected_phones = filter_none(normalised_data([number.to_dict() for number in user.phone_numbers]))
         _obtained_phones = filter_none(normalised_data(response.json().get("phoneNumbers", [])))
         assert _obtained_phones == _expected_phones, 'parsed_response.json().get("phoneNumbers") != user.phone_numbers'
-        assert user.preferred_language == response.json().get("preferredLanguage"), (
-            'user.preferred_language != parsed_response.json().get("preferredLanguage")'
-        )
+        assert user.preferred_language == response.json().get(
+            "preferredLanguage"
+        ), 'user.preferred_language != parsed_response.json().get("preferredLanguage")'
 
         # If the request has NUTID profiles, ensure they are present in the parsed_response
         if SCIMSchema.NUTID_USER_V1.value in req:

src/eduid/webapp/common/api/testing.py

@@ -517,19 +517,19 @@ def _assure_not_in_dict(d: Mapping[str, Any], unwanted_key: str) -> None:
             if message is not None:
                 assert "message" in _json["payload"], 'JSON payload has no "message" element'
                 _message_value = _json["payload"]["message"]
-                assert message.value == _message_value, (
-                    f"Wrong message returned. expected: {message.value}, actual: {_message_value}"
-                )
+                assert (
+                    message.value == _message_value
+                ), f"Wrong message returned. expected: {message.value}, actual: {_message_value}"
             if error is not None:
                 assert _json["error"] is True, "The Flux response was supposed to have error=True"
                 assert "error" in _json["payload"], 'JSON payload has no "error" element'
                 _error = _json["payload"]["error"]
                 assert error == _error, f"Wrong error returned. expected: {error}, actual: {_error}"
             if payload is not None:
                 for k, v in payload.items():
-                    assert k in _json["payload"], (
-                        f"The Flux response payload {_json['payload']} does not contain {repr(k)}"
-                    )
+                    assert (
+                        k in _json["payload"]
+                    ), f"The Flux response payload {_json['payload']} does not contain {repr(k)}"
                     assert v == _json["payload"][k], (
                         f"The Flux response payload item {repr(k)} should be {repr(v)} "
                         f"but is {repr(_json['payload'][k])}"
@@ -540,9 +540,9 @@ def _assure_not_in_dict(d: Mapping[str, Any], unwanted_key: str) -> None:
             if meta is not None:
                 for k, v in meta.items():
                     assert k in _json["meta"], f"The Flux response meta does not contain {repr(k)}"
-                    assert v == _json["meta"][k], (
-                        f"The Flux response meta item {repr(k)} should be {repr(v)} but is {repr(_json['meta'][k])}"
-                    )
+                    assert (
+                        v == _json["meta"][k]
+                    ), f"The Flux response meta item {repr(k)} should be {repr(v)} but is {repr(_json['meta'][k])}"
 
         except (AssertionError, KeyError):
             if response.json:

src/eduid/webapp/common/proofing/testing.py

@@ -77,49 +77,49 @@ def _verify_user_parameters(
         user_mfa_tokens = user.credentials.filter(FidoCredential)
 
         # Check token status
-        assert len(user_mfa_tokens) == num_mfa_tokens, (
-            f"Unexpected number of FidoCredentials on user. {len(user_mfa_tokens)}, expected {num_mfa_tokens}"
-        )
+        assert (
+            len(user_mfa_tokens) == num_mfa_tokens
+        ), f"Unexpected number of FidoCredentials on user. {len(user_mfa_tokens)}, expected {num_mfa_tokens}"
         if user_mfa_tokens:
-            assert user_mfa_tokens[0].is_verified == token_verified, (
-                f"User token was expected to be verified={token_verified}"
-            )
+            assert (
+                user_mfa_tokens[0].is_verified == token_verified
+            ), f"User token was expected to be verified={token_verified}"
 
         _log = getattr(self.app, "proofing_log")
         assert isinstance(_log, ProofingLog)
-        assert _log.db_count() == num_proofings, (
-            f"Unexpected number of proofings in db. {_log.db_count()}, expected {num_proofings}"
-        )
+        assert (
+            _log.db_count() == num_proofings
+        ), f"Unexpected number of proofings in db. {_log.db_count()}, expected {num_proofings}"
 
         if identity is not None:
             # Check parameters of a specific nin
             user_identity = user.identities.find(identity.identity_type)
             if not identity_present:
-                assert (user_identity is None or user_identity.unique_value != identity.unique_value) is True, (
-                    f"identity {identity} not expected to be present on user"
-                )
+                assert (
+                    user_identity is None or user_identity.unique_value != identity.unique_value
+                ) is True, f"identity {identity} not expected to be present on user"
                 return None
             assert user_identity is not None, f"identity {identity} not present on user"
-            assert user_identity.unique_value == identity.unique_value, (
-                "user_identity.unique_value != identity.unique_value"
-            )
-            assert user_identity.is_verified == identity_verified, (
-                f"{user_identity} was expected to be verified={identity_verified}"
-            )
+            assert (
+                user_identity.unique_value == identity.unique_value
+            ), "user_identity.unique_value != identity.unique_value"
+            assert (
+                user_identity.is_verified == identity_verified
+            ), f"{user_identity} was expected to be verified={identity_verified}"
 
             if proofing_method is not None:
-                assert user_identity.proofing_method == proofing_method, (
-                    f"{user_identity.proofing_method} != {proofing_method}"
-                )
+                assert (
+                    user_identity.proofing_method == proofing_method
+                ), f"{user_identity.proofing_method} != {proofing_method}"
             if proofing_version is not None:
-                assert user_identity.proofing_version == proofing_version, (
-                    f"{user_identity.proofing_version} != {proofing_version}"
-                )
+                assert (
+                    user_identity.proofing_version == proofing_version
+                ), f"{user_identity.proofing_version} != {proofing_version}"
 
         if locked_identity is not None:
             # Check parameters of a specific locked nin
             user_locked_identity = user.locked_identity.find(locked_identity.identity_type)
             assert user_locked_identity is not None, f"locked identity {locked_identity} not present"
-            assert user_locked_identity.unique_value == locked_identity.unique_value, (
-                f"locked identity {user_locked_identity.unique_value} not matching {locked_identity.unique_value}"
-            )
+            assert (
+                user_locked_identity.unique_value == locked_identity.unique_value
+            ), f"locked identity {user_locked_identity.unique_value} not matching {locked_identity.unique_value}"

src/eduid/webapp/freja_eid/tests/test_app.py

@@ -342,9 +342,9 @@ def test_verify_identity_request(self) -> None:
         assert query["response_type"] == ["code"]
         assert query["client_id"] == ["test_client_id"]
         assert query["redirect_uri"] == ["http://test.localhost/authn-callback"]
-        assert query["scope"] == [" ".join(self.app.conf.freja_eid_client.scopes)], (
-            f"{query['scope']} != {[' '.join(self.app.conf.freja_eid_client.scopes)]}"
-        )
+        assert query["scope"] == [
+            " ".join(self.app.conf.freja_eid_client.scopes)
+        ], f"{query['scope']} != {[' '.join(self.app.conf.freja_eid_client.scopes)]}"
         assert query["code_challenge_method"] == ["S256"]
 
     @patch("eduid.webapp.common.api.helpers.get_reference_nin_from_navet_data")

src/eduid/webapp/idp/tests/test_SSO.py

@@ -259,9 +259,9 @@ def _check_login_response_authn(
         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}"
-            )
+            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

src/eduid/webapp/idp/tests/test_api.py

@@ -376,9 +376,9 @@ def _call_mfa(
                 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("webauthn_options") == mock_stv.return_value.webauthn_options
+        ), f"webauthn_options: {payload.get('webauthn_options')}, Expected: {mock_stv.return_value.webauthn_options}"
         assert not payload.get("finished"), "Expected finished=False"
 
         logger.debug(f"MFA endpoint returned (challenge):\n{json.dumps(response.json, indent=4)}")
@@ -599,9 +599,9 @@ def _check_login_result(
         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}"
-            )
+            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"

src/eduid/webapp/personal_data/tests/test_app.py

@@ -560,6 +560,6 @@ def test_is_valid_chosen_given_name() -> None:
             ("Sverker Jr", "Jr", True),
         ]
         for param in params:
-            assert is_valid_chosen_given_name(param[0], param[1]) is param[2], (
-                f"{param[0]}, {param[1]} did not return {param[2]}"
-            )
+            assert (
+                is_valid_chosen_given_name(param[0], param[1]) is param[2]
+            ), f"{param[0]}, {param[1]} did not return {param[2]}"

src/eduid/webapp/security/helpers.py

@@ -4,8 +4,6 @@
 from functools import cache
 from typing import Any
 
-from fido_mds.models.webauthn import AttestationFormat
-
 from eduid.common.config.base import EduidEnvironment
 from eduid.common.misc.timeutil import utc_now
 from eduid.common.rpc.msg_relay import FullPostalAddress, NavetData
@@ -21,7 +19,6 @@
 from eduid.webapp.common.api.messages import TranslatableMsg
 from eduid.webapp.common.api.translation import get_user_locale
 from eduid.webapp.security.app import current_security_app as current_app
-from eduid.webapp.security.webauthn_proofing import AuthenticatorInformation, is_authenticator_mfa_approved
 
 __author__ = "lundberg"
 
@@ -240,40 +237,52 @@ def update_user_official_name(security_user: SecurityUser, navet_data: NavetData
 @cache
 def get_approved_security_keys() -> dict[str, Any]:
     # a way to reuse is_authenticator_mfa_approved() from security app
-    parsed_entries: list[AuthenticatorInformation] = []
+    parsed_entries: list[str] = []
     for metadata_entry in current_app.fido_mds.metadata.entries:
         user_verification_methods = [
             detail.user_verification_method
             for detail in metadata_entry.metadata_statement.get_user_verification_details()
         ]
 
-        # simulated to fit AuthenticatorInformation format
-        attestation_format = AttestationFormat.PACKED
-        if not metadata_entry.metadata_statement.attestation_types:
-            attestation_format = AttestationFormat.NONE
-
-        authenticator_info = AuthenticatorInformation(
-            authenticator_id=metadata_entry.aaguid or metadata_entry.aaid,
-            attestation_format=attestation_format,
-            user_present=False,  # simulated to fit AuthenticatorInformation required fields
-            user_verified=False,  # simulated to fit AuthenticatorInformation required fields
-            status=metadata_entry.status_reports[0].status,
-            last_status_change=metadata_entry.time_of_last_status_change,
-            user_verification_methods=user_verification_methods,
-            key_protection=metadata_entry.metadata_statement.key_protection,
-            description=metadata_entry.metadata_statement.description,
+        # check latest status in metadata and disallow incident statuses
+        # TODO: user latest_status_report method on Entry when available in python-fido-mds
+        latest_status_report = max(metadata_entry.status_reports, key=lambda sr: sr.effective_date)
+        if latest_status_report.status in current_app.conf.webauthn_disallowed_status:
+            current_app.logger.debug(f"status {latest_status_report.status} is not mfa capable")
+            continue
+
+        # true if the authenticator supports any of the user verification methods we allow
+        is_accepted_user_verification = any(
+            [
+                method
+                for method in user_verification_methods
+                if method in current_app.conf.webauthn_recommended_user_verification_methods
+            ]
         )
-        parsed_entries.append(authenticator_info)
-
-    approved_keys_list: list[str] = [
-        entry.description for entry in parsed_entries if entry.description and is_authenticator_mfa_approved(entry)
-    ]
+        # a typical token has key protection ["hardware"] or ["hardware", "tee"] but some also support software, so
+        # we have to check that all key protections supported is in our allow list
+        is_accepted_key_protection = all(
+            [
+                protection
+                for protection in metadata_entry.metadata_statement.key_protection
+                if protection in current_app.conf.webauthn_recommended_key_protection
+            ]
+        )
+        current_app.logger.debug(f"security key description: {metadata_entry.metadata_statement.description}")
+        current_app.logger.debug(f"is_accepted_user_verification: {is_accepted_user_verification}")
+        current_app.logger.debug(f"is_accepted_key_protection: {is_accepted_key_protection}")
+        if (
+            is_accepted_user_verification
+            and is_accepted_key_protection
+            and metadata_entry.metadata_statement.description
+        ):
+            parsed_entries.append(metadata_entry.metadata_statement.description)
 
     # remove case-insensitive duplicates from list, while maintaining the original case
     marker = set()
     unique_approved_keys_list: list[str] = []
 
-    for key in approved_keys_list:
+    for key in parsed_entries:
         lower_case_key = key.lower()
         if lower_case_key not in marker:  # test presence
             marker.add(lower_case_key)

src/eduid/webapp/security/settings/common.py

@@ -42,11 +42,11 @@ class SecurityConfig(
 
     # webauthn
     webauthn_proofing_method: str = Field(default="webauthn metadata")
-    webauthn_proofing_version: str = Field(default="2022v1")
+    webauthn_proofing_version: str = Field(default="2024v1")
     webauthn_max_allowed_tokens: int = 10
     webauthn_attestation: AttestationConveyancePreference | None = None
     webauthn_user_verification: UserVerificationRequirement = UserVerificationRequirement.PREFERRED
-    webauthn_allowed_user_verification_methods: list[str] = Field(
+    webauthn_recommended_user_verification_methods: list[str] = Field(
         default=[
             "faceprint_internal",
             "passcode_external",
@@ -59,18 +59,16 @@ class SecurityConfig(
             "apple",
         ]
     )
-    webauthn_allowed_key_protection: list[str] = Field(
+    webauthn_recommended_key_protection: list[str] = Field(
         default=["remote_handle", "hardware", "secure_element", "tee", "apple"]
     )
-    webauthn_allowed_status: list[AuthenticatorStatus] = Field(
+    webauthn_disallowed_status: list[AuthenticatorStatus] = Field(
         default=[
-            AuthenticatorStatus.FIDO_CERTIFIED,
-            AuthenticatorStatus.FIDO_CERTIFIED_L1,
-            AuthenticatorStatus.FIDO_CERTIFIED_L2,
-            AuthenticatorStatus.FIDO_CERTIFIED_L3,
-            AuthenticatorStatus.FIDO_CERTIFIED_L1plus,
-            AuthenticatorStatus.FIDO_CERTIFIED_L2plus,
-            AuthenticatorStatus.FIDO_CERTIFIED_L3plus,
+            AuthenticatorStatus.USER_VERIFICATION_BYPASS,
+            AuthenticatorStatus.ATTESTATION_KEY_COMPROMISE,
+            AuthenticatorStatus.USER_KEY_REMOTE_COMPROMISE,
+            AuthenticatorStatus.USER_KEY_PHYSICAL_COMPROMISE,
+            AuthenticatorStatus.REVOKED,
         ]
     )