From 52c3bf76afc5820d74e76405e0033976dd16b492 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Wed, 4 Dec 2024 17:00:00 -0800 Subject: [PATCH 1/3] Don't set attachment based on hints --- _app/homepage/services/registration.py | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/_app/homepage/services/registration.py b/_app/homepage/services/registration.py index 0c827c0..a4157f5 100644 --- a/_app/homepage/services/registration.py +++ b/_app/homepage/services/registration.py @@ -56,28 +56,7 @@ def generate_registration_options( user_verification=UserVerificationRequirement.DISCOURAGED, resident_key=ResidentKeyRequirement.PREFERRED, ) - if len(hints) > 0: - """ - Deferring to hints when present as per https://w3c.github.io/webauthn/#enum-hints - """ - if hints[0] == "security-key": - # "For compatibility with older user agents, when this hint is used in - # PublicKeyCredentialCreationOptions, the authenticatorAttachment SHOULD be set to - # cross-platform." - authenticator_attachment = AuthenticatorAttachment.CROSS_PLATFORM - elif hints[0] == "hybrid": - # "For compatibility with older user agents, when this hint is used in - # PublicKeyCredentialCreationOptions, the authenticatorAttachment SHOULD be set to - # cross-platform." - authenticator_attachment = AuthenticatorAttachment.CROSS_PLATFORM - elif hints[0] == "client-device": - # "For compatibility with older user agents, when this hint is used in - # PublicKeyCredentialCreationOptions, the authenticatorAttachment SHOULD be set to - # platform." - authenticator_attachment = AuthenticatorAttachment.PLATFORM - - authenticator_selection.authenticator_attachment = authenticator_attachment - elif attachment != "all": + if attachment != "all": authenticator_attachment = AuthenticatorAttachment.CROSS_PLATFORM if attachment == "platform": authenticator_attachment = AuthenticatorAttachment.PLATFORM From 88bc48fb0b91699fcdd9bc45960eeeef217d9bee Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 5 Dec 2024 11:07:28 -0800 Subject: [PATCH 2/3] Add tests --- .../tests/test_registration_service.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/_app/homepage/tests/test_registration_service.py b/_app/homepage/tests/test_registration_service.py index 1a6f74d..99d13e5 100644 --- a/_app/homepage/tests/test_registration_service.py +++ b/_app/homepage/tests/test_registration_service.py @@ -94,3 +94,27 @@ def test_parse_hints(self) -> None: PublicKeyCredentialHint.HYBRID, ], ) + + def test_hints_do_not_change_attachment(self) -> None: + options = self.service.generate_registration_options( + username="mmiller", + algorithms=["ed25519", "es256"], + attestation="direct", + discoverable_credential="required", + existing_credentials=[], + user_verification="discouraged", + # Sometimes it's useful to test conflicting options like this to gauge browser behavior + attachment="platform", + hints=["security-key", "hybrid"], + ) + + assert options.authenticator_selection + + self.assertEqual( + options.authenticator_selection.authenticator_attachment, + AuthenticatorAttachment.PLATFORM, + ) + self.assertEqual( + options.hints, + [PublicKeyCredentialHint.SECURITY_KEY, PublicKeyCredentialHint.HYBRID], + ) From 85fb75d6afa262915a24a8f2c1d99318d3c9f008 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Thu, 5 Dec 2024 11:44:16 -0800 Subject: [PATCH 3/3] Add tests --- .../tests/test_registration_service.py | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/_app/homepage/tests/test_registration_service.py b/_app/homepage/tests/test_registration_service.py index 99d13e5..a01241d 100644 --- a/_app/homepage/tests/test_registration_service.py +++ b/_app/homepage/tests/test_registration_service.py @@ -95,6 +95,63 @@ def test_parse_hints(self) -> None: ], ) + def test_set_attachment_platform(self) -> None: + options = self.service.generate_registration_options( + username="mmiller", + algorithms=["ed25519", "es256"], + attestation="direct", + discoverable_credential="required", + existing_credentials=[], + user_verification="discouraged", + hints=["client-device", "security-key", "hybrid"], + # Support security keys and hybrid + attachment="cross-platform", + ) + + assert options.authenticator_selection + + self.assertEqual( + options.authenticator_selection.authenticator_attachment, + AuthenticatorAttachment.CROSS_PLATFORM, + ) + + def test_set_attachment_cross_platform(self) -> None: + options = self.service.generate_registration_options( + username="mmiller", + algorithms=["ed25519", "es256"], + attestation="direct", + discoverable_credential="required", + existing_credentials=[], + user_verification="discouraged", + hints=["client-device", "security-key", "hybrid"], + # Support only platform authenticators + attachment="platform", + ) + + assert options.authenticator_selection + + self.assertEqual( + options.authenticator_selection.authenticator_attachment, + AuthenticatorAttachment.PLATFORM, + ) + + def test_set_attachment_all(self) -> None: + options = self.service.generate_registration_options( + username="mmiller", + algorithms=["ed25519", "es256"], + attestation="direct", + discoverable_credential="required", + existing_credentials=[], + user_verification="discouraged", + hints=[], + # Support everything + attachment="all", + ) + + assert options.authenticator_selection + + self.assertIsNone(options.authenticator_selection.authenticator_attachment) + def test_hints_do_not_change_attachment(self) -> None: options = self.service.generate_registration_options( username="mmiller",