Merge pull request #150 from duo-labs/fix/decouple-reg-hints-and-atta…

…chment

fix/decouple-reg-hints-and-attachment

_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

_app/homepage/tests/test_registration_service.py

@@ -94,3 +94,84 @@ def test_parse_hints(self) -> None:
                 PublicKeyCredentialHint.HYBRID,
             ],
         )
+
+    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",
+            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],
+        )