diff --git a/src/eduid/scimapi/testing.py b/src/eduid/scimapi/testing.py
index 7f5ac69cc..b1df50655 100644
--- a/src/eduid/scimapi/testing.py
+++ b/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}"
diff --git a/src/eduid/scimapi/tests/test_scimuser.py b/src/eduid/scimapi/tests/test_scimuser.py
index a1f4d9bce..38f603d14 100644
--- a/src/eduid/scimapi/tests/test_scimuser.py
+++ b/src/eduid/scimapi/tests/test_scimuser.py
@@ -204,9 +204,9 @@ 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", [])))
@@ -214,9 +214,9 @@ def _assertUserUpdateSuccess(self, req: Mapping, response: Response, user: ScimA
_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:
diff --git a/src/eduid/webapp/common/api/testing.py b/src/eduid/webapp/common/api/testing.py
index d8539f9b9..4658a0a6e 100644
--- a/src/eduid/webapp/common/api/testing.py
+++ b/src/eduid/webapp/common/api/testing.py
@@ -517,9 +517,9 @@ 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'
@@ -527,9 +527,9 @@ def _assure_not_in_dict(d: Mapping[str, Any], unwanted_key: str) -> None:
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:
diff --git a/src/eduid/webapp/common/proofing/testing.py b/src/eduid/webapp/common/proofing/testing.py
index 67c159b2c..e64bd0c66 100644
--- a/src/eduid/webapp/common/proofing/testing.py
+++ b/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}"
diff --git a/src/eduid/webapp/freja_eid/tests/test_app.py b/src/eduid/webapp/freja_eid/tests/test_app.py
index a7fd4da1c..62e8a5381 100644
--- a/src/eduid/webapp/freja_eid/tests/test_app.py
+++ b/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")
diff --git a/src/eduid/webapp/idp/tests/test_SSO.py b/src/eduid/webapp/idp/tests/test_SSO.py
index b6fd3e668..5d7cc49c5 100644
--- a/src/eduid/webapp/idp/tests/test_SSO.py
+++ b/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
diff --git a/src/eduid/webapp/idp/tests/test_api.py b/src/eduid/webapp/idp/tests/test_api.py
index 03507ff87..359269e1c 100644
--- a/src/eduid/webapp/idp/tests/test_api.py
+++ b/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"
diff --git a/src/eduid/webapp/personal_data/tests/test_app.py b/src/eduid/webapp/personal_data/tests/test_app.py
index 4befd46c0..c7862a28b 100644
--- a/src/eduid/webapp/personal_data/tests/test_app.py
+++ b/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]}"
diff --git a/src/eduid/webapp/security/helpers.py b/src/eduid/webapp/security/helpers.py
index 5de3c9239..13041150b 100644
--- a/src/eduid/webapp/security/helpers.py
+++ b/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)
diff --git a/src/eduid/webapp/security/settings/common.py b/src/eduid/webapp/security/settings/common.py
index ce1bb1c7b..ef63448c1 100644
--- a/src/eduid/webapp/security/settings/common.py
+++ b/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,
]
)
diff --git a/src/eduid/webapp/security/tests/test_webauthn.py b/src/eduid/webapp/security/tests/test_webauthn.py
index 432cfd428..f76a41bcc 100644
--- a/src/eduid/webapp/security/tests/test_webauthn.py
+++ b/src/eduid/webapp/security/tests/test_webauthn.py
@@ -65,34 +65,27 @@
)
# CTAP2 security key
-STATE_2 = {"challenge": "iW6wn2xAYUfBueKvhIyTsB6YRsQz9OIwaPfw1ZoCtNY", "user_verification": "discouraged"}
-
-ATTESTATION_OBJECT_2 = (
- b"o2NmbXRmcGFja2VkZ2F0dFN0bXSjY2FsZyZjc2lnWEcwRQIhANNvwZhaTvdSujKW3pUCfeYB_ABjCo2X"
- b"dg8e5RowhAgZAiBmj8DH71y46Rg9W67BTG1MuBmaycK7osVy6g_ppmJGiGN4NWOBWQLBMIICvTCCAaWg"
- b"AwIBAgIEHo-HNDANBgkqhkiG9w0BAQsFADAuMSwwKgYDVQQDEyNZdWJpY28gVTJGIFJvb3QgQ0EgU2Vy"
- b"aWFsIDQ1NzIwMDYzMTAgFw0xNDA4MDEwMDAwMDBaGA8yMDUwMDkwNDAwMDAwMFowbjELMAkGA1UEBhMC"
- b"U0UxEjAQBgNVBAoMCVl1YmljbyBBQjEiMCAGA1UECwwZQXV0aGVudGljYXRvciBBdHRlc3RhdGlvbjEn"
- b"MCUGA1UEAwweWXViaWNvIFUyRiBFRSBTZXJpYWwgNTEyNzIyNzQwMFkwEwYHKoZIzj0CAQYIKoZIzj0D"
- b"AQcDQgAEqHn4IzjtFJS6wHBLzH_GY9GycXFZdiQxAcdgURXXwVKeKBwcZzItOEtc1V3T6YGNX9hcIq8y"
- b"bgxk_CCv4z8jZqNsMGowIgYJKwYBBAGCxAoCBBUxLjMuNi4xLjQuMS40MTQ4Mi4xLjcwEwYLKwYBBAGC"
- b"5RwCAQEEBAMCBDAwIQYLKwYBBAGC5RwBAQQEEgQQL8BXn4ETR-qxFrtajbkgKjAMBgNVHRMBAf8EAjAA"
- b"MA0GCSqGSIb3DQEBCwUAA4IBAQCGk_9i3w1XedR0jX_I0QInMYqOWA5qOlfBCOlOA8OFaLNmiU_OViS-"
- b"Sj79fzQRiz2ZN0P3kqGYkWDI_JrgsE49-e4V4-iMBPyCqNy_WBjhCNzCloV3rnn_ZiuUc0497EWXMF1z"
- b"5uVe4r65zZZ4ygk15TPrY4-OJvq7gXzaRB--mDGDKuX24q2ZL56720xiI4uPjXq0gdbTJjvNv55KV1UD"
- b"cJiK1YE0QPoDLK22cjyt2PjXuoCfdbQ8_6Clua3RQjLvnZ4UgSY4IzxMpKhzufismOMroZFnYG4VkJ_N"
- b"20ot_72uRiAkn5pmRqyB5IMtERn-v6pzGogtolp3gn1G0ZAXaGF1dGhEYXRhWMTc9wRxJipqDOIk0QJj"
- b"FIEoxHTUnimbJIcg88Mz7jZh00EAAAAEL8BXn4ETR-qxFrtajbkgKgBAz6lLs6rFz6zm4IH73RUcSaVb"
- b"C5v4-J6j8HGS-VwPYIhyYUi6d1mxHYZPuehFrC5-r4ZmFqd7gpMdoJCo4H1bT6UBAgMmIAEhWCBTzCFk"
- b"fHJcW7ny9bUS4h8YqUadyw0q4kg91vgkScDscCJYICsxt49Uv09cN4OSw93GtjMLxgoaIfFhAK0vd9WL"
- b"vOY8"
-)
-
-CLIENT_DATA_JSON_2 = (
- b"eyJ0eXBlIjoid2ViYXV0aG4uY3JlYXRlIiwiY2hhbGxlbmdlIjoiaVc2d24yeEFZVWZCdWVLdmhJeVRz"
- b"QjZZUnNRejlPSXdhUGZ3MVpvQ3ROWSIsIm9yaWdpbiI6Imh0dHBzOi8vZGFzaGJvYXJkLmVkdWlkLmRv"
- b"Y2tlciIsImNyb3NzT3JpZ2luIjpmYWxzZX0"
-)
+STATE_2 = {"challenge": "yxHWG+ouoa8MLGSxOJJhM0NtiH5ubK3BPfx+6uE3QkU=", "user_verification": "required"}
+
+ATTESTATION_OBJECT_2 = b"""
+o2NmbXRmcGFja2VkZ2F0dFN0bXSjY2FsZyZjc2lnWEYwRAIgHFOTISd2NExUtGr1GTkgImVIJx09yJfKdx7j1f714r0CIF6vTppPv1mRyism5kZjMq+pEvi3
+BATxv2m/kRvlD5zhY3g1Y4FZAsEwggK9MIIBpaADAgECAgQej4c0MA0GCSqGSIb3DQEBCwUAMC4xLDAqBgNVBAMTI1l1YmljbyBVMkYgUm9vdCBDQSBTZXJp
+YWwgNDU3MjAwNjMxMCAXDTE0MDgwMTAwMDAwMFoYDzIwNTAwOTA0MDAwMDAwWjBuMQswCQYDVQQGEwJTRTESMBAGA1UECgwJWXViaWNvIEFCMSIwIAYDVQQL
+DBlBdXRoZW50aWNhdG9yIEF0dGVzdGF0aW9uMScwJQYDVQQDDB5ZdWJpY28gVTJGIEVFIFNlcmlhbCA1MTI3MjI3NDAwWTATBgcqhkjOPQIBBggqhkjOPQMB
+BwNCAASoefgjOO0UlLrAcEvMf8Zj0bJxcVl2JDEBx2BRFdfBUp4oHBxnMi04S1zVXdPpgY1f2FwirzJuDGT8IK/jPyNmo2wwajAiBgkrBgEEAYLECgIEFTEu
+My42LjEuNC4xLjQxNDgyLjEuNzATBgsrBgEEAYLlHAIBAQQEAwIEMDAhBgsrBgEEAYLlHAEBBAQSBBAvwFefgRNH6rEWu1qNuSAqMAwGA1UdEwEB/wQCMAAw
+DQYJKoZIhvcNAQELBQADggEBAIaT/2LfDVd51HSNf8jRAicxio5YDmo6V8EI6U4Dw4Vos2aJT85WJL5KPv1/NBGLPZk3Q/eSoZiRYMj8muCwTj357hXj6IwE
+/IKo3L9YGOEI3MKWhXeuef9mK5RzTj3sRZcwXXPm5V7ivrnNlnjKCTXlM+tjj44m+ruBfNpEH76YMYMq5fbirZkvnrvbTGIji4+NerSB1tMmO82/nkpXVQNw
+mIrVgTRA+gMsrbZyPK3Y+Ne6gJ91tDz/oKW5rdFCMu+dnhSBJjgjPEykqHO5+KyY4yuhkWdgbhWQn83bSi3/va5GICSfmmZGrIHkgy0RGf6/qnMaiC2iWneC
+fUbRkBdoYXV0aERhdGFYxNz3BHEmKmoM4iTRAmMUgSjEdNSeKZskhyDzwzPuNmHTRQAAAAIvwFefgRNH6rEWu1qNuSAqAEDl1oXc7ZWmdlYmRhcoe5htjx6D
++mBRxGkyn3o/xCQfq0FrCveEmEHkG9YmfOxgM77SQrRfG9o3jHuZfpEBLZwopQECAyYgASFYIAFrblN2QOoc1mGVVyS5SvjcQsg2aDZrqfHnrb0iTTiJIlgg
+PJvxSmTxEspL+STelnGuWKgEIiAGR9S+snHAGqkaYf4=
+"""
+
+CLIENT_DATA_JSON_2 = b"""
+eyJ0eXBlIjoid2ViYXV0aG4uY3JlYXRlIiwiY2hhbGxlbmdlIjoieXhIV0ctb3VvYThNTEdTeE9KSmhNME50aUg1dWJLM0JQZngtNnVFM1FrVSIsIm9yaWdp
+biI6Imh0dHBzOi8vaHRtbC5lZHVpZC5kb2NrZXIiLCJjcm9zc09yaWdpbiI6ZmFsc2V9
+"""
CREDENTIAL_ID_2 = (
"7bad3b59fa16e9e8840e2fd15c026a3c9a7d2877fd7d97c25a3b3158d1a9cba1e14b7c9913915"
@@ -637,9 +630,9 @@ def test_authenticator_information(self) -> None:
with self.app.test_request_context():
res = is_authenticator_mfa_approved(authenticator_info=authenticator_info)
- if authenticator in [YUBIKEY_4, NONE_ATTESTATION]:
+ if authenticator in [YUBIKEY_4, YUBIKEY_5_NFC]:
# Yubikey 4 does not support any user verification we accept
- # None attestations cannot be verified to support anything we accept
+ # The test data for Yubikey 5 do not include user verification
assert res is False
else:
assert res is True
@@ -689,7 +682,7 @@ def test_authenticator_information_backdoor(self) -> None:
with self.app.test_request_context():
res = is_authenticator_mfa_approved(authenticator_info=authenticator_info)
- assert res is False
+ assert res is True
def test_approved_security_keys(self) -> None:
response = self.browser.get("/webauthn/approved-security-keys")
diff --git a/src/eduid/webapp/security/views/webauthn.py b/src/eduid/webapp/security/views/webauthn.py
index 38ef36acd..da4c99fb0 100644
--- a/src/eduid/webapp/security/views/webauthn.py
+++ b/src/eduid/webapp/security/views/webauthn.py
@@ -12,7 +12,7 @@
CollectedClientData,
PublicKeyCredentialUserEntity,
)
-from fido_mds.exceptions import AttestationVerificationError, MetadataValidationError
+from fido_mds.exceptions import AttestationVerificationError
from flask import Blueprint
from eduid.common.config.base import FrontendAction
@@ -167,11 +167,6 @@ def registration_complete(
current_app.logger.info(f"attestation_object: {attestation_object}")
current_app.logger.info(f"client_data: {client_data}")
return error_response(message=SecurityMsg.webauthn_attestation_fail)
- except MetadataValidationError:
- current_app.logger.exception("metadata validation failed")
- current_app.logger.info(f"attestation_object: {attestation_object}")
- current_app.logger.info(f"client_data: {client_data}")
- return error_response(message=SecurityMsg.webauthn_metadata_fail)
# Move registration state from session to local variable to let users restart if something fails
reg_state = session.security.webauthn_registration
diff --git a/src/eduid/webapp/security/webauthn_proofing.py b/src/eduid/webapp/security/webauthn_proofing.py
index 1731b34dc..ab19f7b07 100644
--- a/src/eduid/webapp/security/webauthn_proofing.py
+++ b/src/eduid/webapp/security/webauthn_proofing.py
@@ -74,11 +74,25 @@ def get_authenticator_information(attestation: str, client_data: str) -> Authent
# verify attestation
try:
current_app.fido_mds.verify_attestation(attestation=att, client_data=websafe_decode(client_data))
- except (AttestationVerificationError, MetadataValidationError) as e:
+ except AttestationVerificationError as e:
current_app.logger.debug(f"attestation: {att}")
current_app.logger.debug(f"client_data: {client_data}")
current_app.logger.exception("Failed to get authenticator information")
raise e
+ except MetadataValidationError:
+ current_app.logger.debug(f"attestation: {att}")
+ current_app.logger.debug(f"client_data: {client_data}")
+ current_app.logger.exception(
+ "Failed to get authenticator information from metadata, continuing without metadata"
+ )
+ # Continue even if the security key _should_ be found and validated with metadata as we have seen security keys
+ # in the wild that fails
+ return AuthenticatorInformation(
+ attestation_format=AttestationFormat.NONE,
+ authenticator_id=authenticator_id,
+ user_present=user_present,
+ user_verified=user_verified,
+ )
# There are no metadata entries for Apple devices, just create the authenticator information
if att.fmt is AttestationFormat.APPLE:
@@ -119,7 +133,9 @@ def get_authenticator_information(attestation: str, client_data: str) -> Authent
return AuthenticatorInformation(
attestation_format=att.fmt,
authenticator_id=att.aaguid or att.certificate_key_identifier,
- status=metadata_entry.status_reports[0].status, # latest status reports status
+ status=max(
+ metadata_entry.status_reports, key=lambda sr: sr.effective_date
+ ).status, # latest status reports status
last_status_change=last_status_change,
user_verification_methods=user_verification_methods,
key_protection=metadata_entry.metadata_statement.key_protection,
@@ -134,54 +150,25 @@ def is_authenticator_mfa_approved(authenticator_info: AuthenticatorInformation)
"""
This is our current policy for determine if a FIDO2 authenticator can do multi-factor authentications.
"""
- # If there is no attestation we can not trust the authenticator info
- if authenticator_info.attestation_format == AttestationFormat.NONE:
- return False
+ if authenticator_info.user_verified:
+ current_app.logger.info(
+ f"AttestationFormat {authenticator_info.attestation_format}: user verified - authenticator is mfa capable"
+ )
- # Our current policy is that Apple is capable of mfa
- if authenticator_info.status is OtherAuthenticatorStatus.APPLE:
- current_app.logger.debug("apple device is mfa capable")
- return True
+ # check status in metadata (if any) and disallow incident statuses
+ if authenticator_info.status and authenticator_info.status in current_app.conf.webauthn_disallowed_status:
+ current_app.logger.debug(f"status {authenticator_info.status} is not mfa capable")
+ return False
- # check status in metadata and disallow uncertified and incident statuses
- if authenticator_info.status not in current_app.conf.webauthn_allowed_status:
- current_app.logger.debug(f"status {authenticator_info.status} is not mfa capable")
- return False
-
- # true if the authenticator supports any of the user verification methods we allow
- is_accepted_user_verification = any(
- [
- method
- for method in authenticator_info.user_verification_methods
- if method in current_app.conf.webauthn_allowed_user_verification_methods
- ]
- )
- # 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 authenticator_info.key_protection
- if protection in current_app.conf.webauthn_allowed_key_protection
- ]
- )
- 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:
return True
+ current_app.logger.info(
+ f"AttestationFormat {authenticator_info.attestation_format}: user NOT verified - authenticator is not mfa "
+ f"capable"
+ )
return False
def save_webauthn_proofing_log(eppn: str, authenticator_info: AuthenticatorInformation) -> bool:
- user_verification_methods_match = set(authenticator_info.user_verification_methods) & set(
- current_app.conf.webauthn_allowed_user_verification_methods
- )
- current_app.logger.debug(f"user verifications methods that match config: {user_verification_methods_match}")
- key_protection_match = set(authenticator_info.key_protection) & set(
- current_app.conf.webauthn_allowed_key_protection
- )
- current_app.logger.debug(f"user verifications methods that match config: {user_verification_methods_match}")
-
proofing_element = WebauthnMfaCapabilityProofingLog(
created_by=current_app.conf.app_name,
eppn=eppn,
@@ -189,8 +176,8 @@ def save_webauthn_proofing_log(eppn: str, authenticator_info: AuthenticatorInfor
proofing_method=current_app.conf.webauthn_proofing_method,
authenticator_id=authenticator_info.authenticator_id,
attestation_format=authenticator_info.attestation_format,
- user_verification_methods=list(user_verification_methods_match),
- key_protection=list(key_protection_match),
+ user_verification_methods=authenticator_info.user_verification_methods,
+ key_protection=authenticator_info.key_protection,
)
current_app.logger.debug(f"webauthn mfa capability proofing element: {proofing_element}")
return current_app.proofing_log.save(proofing_element)
diff --git a/src/eduid/webapp/signup/tests/test_app.py b/src/eduid/webapp/signup/tests/test_app.py
index c0c19afaa..44f46e01a 100644
--- a/src/eduid/webapp/signup/tests/test_app.py
+++ b/src/eduid/webapp/signup/tests/test_app.py
@@ -744,9 +744,9 @@ def _accept_invite(
assert response.json is not None, "response.json unexpected None"
if expect_success:
- assert response.json.get("error", False) is False, (
- f"expect_success {expect_success} but got error={response.json.get('error')}"
- )
+ assert (
+ response.json.get("error", False) is False
+ ), f"expect_success {expect_success} but got error={response.json.get('error')}"
if not expected_payload:
payload = self.get_response_payload(response)
assert payload["state"]["tou"]["completed"] is False
diff --git a/src/eduid/workers/am/tests/test_proofing_fetchers.py b/src/eduid/workers/am/tests/test_proofing_fetchers.py
index 126f91511..6c83dd976 100644
--- a/src/eduid/workers/am/tests/test_proofing_fetchers.py
+++ b/src/eduid/workers/am/tests/test_proofing_fetchers.py
@@ -94,16 +94,16 @@ def test_append_attributes_letter_proofing_data(self) -> None:
"$unset": {"nins": None},
}
- assert normalised_data(fetched) == expected, (
- f"Fetched letter proofing data has unexpected data: {DeepDiff(fetched, expected)}"
- )
+ assert (
+ normalised_data(fetched) == expected
+ ), f"Fetched letter proofing data has unexpected data: {DeepDiff(fetched, expected)}"
fetched2 = self.fetcher.fetch_attrs(proofing_user.user_id)
# Don't repeat the letter_proofing_data
- assert normalised_data(fetched2) == expected, (
- f"Fetched (2nd time) letter proofing data has unexpected data: {DeepDiff(fetched, expected)}"
- )
+ assert (
+ normalised_data(fetched2) == expected
+ ), f"Fetched (2nd time) letter proofing data has unexpected data: {DeepDiff(fetched, expected)}"
# Adding a new letter_proofing_data
self.user_data["letter_proofing_data"].append(