Merge pull request #197 from duo-labs/feat/user_id_bytes_only

feat/user_id_bytes_only

tests/test_generate_registration_options.py

@@ -19,11 +19,12 @@ class TestGenerateRegistrationOptions(TestCase):
     @patch("secrets.token_bytes")
     def test_generates_options_with_defaults(self, token_bytes_mock: MagicMock) -> None:
         token_bytes_mock.return_value = b"12345"
+        user_id = "ABAV6QWPBEY9WOTOA1A4".encode("utf-8")
 
         options = generate_registration_options(
             rp_id="example.com",
             rp_name="Example Co",
-            user_id="ABAV6QWPBEY9WOTOA1A4",
+            user_id=user_id,
             user_name="lee",
         )
 
@@ -33,7 +34,7 @@ def test_generates_options_with_defaults(self, token_bytes_mock: MagicMock) -> N
         )
         assert options.challenge == b"12345"
         assert options.user == PublicKeyCredentialUserEntity(
-            id=b"ABAV6QWPBEY9WOTOA1A4",
+            id=user_id,
             name="lee",
             display_name="lee",
         )
@@ -47,10 +48,12 @@ def test_generates_options_with_defaults(self, token_bytes_mock: MagicMock) -> N
         assert options.attestation == AttestationConveyancePreference.NONE
 
     def test_generates_options_with_custom_values(self) -> None:
+        user_id = "ABAV6QWPBEY9WOTOA1A4".encode("utf-8")
+
         options = generate_registration_options(
             rp_id="example.com",
             rp_name="Example Co",
-            user_id="ABAV6QWPBEY9WOTOA1A4",
+            user_id=user_id,
             user_name="lee",
             user_display_name="Lee",
             attestation=AttestationConveyancePreference.DIRECT,
@@ -69,7 +72,7 @@ def test_generates_options_with_custom_values(self) -> None:
         assert options.rp == PublicKeyCredentialRpEntity(id="example.com", name="Example Co")
         assert options.challenge == b"1234567890"
         assert options.user == PublicKeyCredentialUserEntity(
-            id=b"ABAV6QWPBEY9WOTOA1A4",
+            id=user_id,
             name="lee",
             display_name="Lee",
         )
@@ -91,7 +94,6 @@ def test_raises_on_empty_rp_id(self) -> None:
             generate_registration_options(
                 rp_id="",
                 rp_name="Example Co",
-                user_id="blah",
                 user_name="blah",
             )
 
@@ -100,24 +102,35 @@ def test_raises_on_empty_rp_name(self) -> None:
             generate_registration_options(
                 rp_id="example.com",
                 rp_name="",
-                user_id="blah",
                 user_name="blah",
             )
 
-    def test_raises_on_empty_user_id(self) -> None:
+    @patch("secrets.token_bytes")
+    def test_generated_random_id_on_empty_user_id(self, token_bytes_mock: MagicMock) -> None:
+        token_bytes_mock.return_value = bytes([1, 2, 3, 4])
+
+        options = generate_registration_options(
+            rp_id="example.com",
+            rp_name="Example Co",
+            user_name="blah",
+            user_id=None,
+        )
+
+        self.assertEqual(options.user.id, bytes([1, 2, 3, 4]))
+
+    def test_raises_on_non_bytes_user_id(self) -> None:
         with self.assertRaisesRegex(ValueError, "user_id"):
             generate_registration_options(
                 rp_id="example.com",
                 rp_name="Example Co",
-                user_id="",
-                user_name="blah",
+                user_name="hello",
+                user_id="hello",  # type: ignore
             )
 
     def test_raises_on_empty_user_name(self) -> None:
         with self.assertRaisesRegex(ValueError, "user_name"):
             generate_registration_options(
                 rp_id="example.com",
                 rp_name="Example Co",
-                user_id="blah",
                 user_name="",
             )

tests/test_options_to_json.py

@@ -22,7 +22,7 @@ def test_converts_registration_options_to_JSON(self) -> None:
         options = generate_registration_options(
             rp_id="example.com",
             rp_name="Example Co",
-            user_id="ABAV6QWPBEY9WOTOA1A4",
+            user_id=bytes([1, 2, 3, 4]),
             user_name="lee",
             user_display_name="Lee",
             attestation=AttestationConveyancePreference.DIRECT,
@@ -45,7 +45,7 @@ def test_converts_registration_options_to_JSON(self) -> None:
             {
                 "rp": {"name": "Example Co", "id": "example.com"},
                 "user": {
-                    "id": "QUJBVjZRV1BCRVk5V09UT0ExQTQ",
+                    "id": "AQIDBA",
                     "name": "lee",
                     "displayName": "Lee",
                 },
@@ -67,7 +67,6 @@ def test_includes_optional_value_when_set(self) -> None:
         options = generate_registration_options(
             rp_id="example.com",
             rp_name="Example Co",
-            user_id="ABAV6QWPBEY9WOTOA1A4",
             user_name="lee",
             exclude_credentials=[
                 PublicKeyCredentialDescriptor(

tests/test_parse_authentication_credential_json.py

@@ -146,7 +146,7 @@ def test_handles_user_handle(self) -> None:
             }
         )
 
-        self.assertEqual(parsed.response.user_handle, "bW1pbGxlcg")
+        self.assertEqual(parsed.response.user_handle, base64url_to_bytes("bW1pbGxlcg"))
 
     def test_handles_missing_user_handle(self) -> None:
         parsed = parse_authentication_credential_json(
@@ -166,7 +166,7 @@ def test_handles_missing_user_handle(self) -> None:
 
     def test_raises_on_non_base64url_raw_id(self) -> None:
         with self.assertRaisesRegex(
-            InvalidAuthenticationResponse, "Unable to parse authentication credential"
+            InvalidAuthenticationResponse, "Could not parse authentication credential"
         ):
             parse_authentication_credential_json(
                 {
@@ -183,7 +183,7 @@ def test_raises_on_non_base64url_raw_id(self) -> None:
 
     def test_raises_on_non_base64url_authenticator_data(self) -> None:
         with self.assertRaisesRegex(
-            InvalidAuthenticationResponse, "Unable to parse authentication credential"
+            InvalidAuthenticationResponse, "Could not parse authentication credential"
         ):
             parse_authentication_credential_json(
                 {
@@ -200,7 +200,7 @@ def test_raises_on_non_base64url_authenticator_data(self) -> None:
 
     def test_raises_on_non_base64url_client_data_json(self) -> None:
         with self.assertRaisesRegex(
-            InvalidAuthenticationResponse, "Unable to parse authentication credential"
+            InvalidAuthenticationResponse, "Could not parse authentication credential"
         ):
             parse_authentication_credential_json(
                 {
@@ -217,7 +217,7 @@ def test_raises_on_non_base64url_client_data_json(self) -> None:
 
     def test_raises_on_non_base64url_signature(self) -> None:
         with self.assertRaisesRegex(
-            InvalidAuthenticationResponse, "Unable to parse authentication credential"
+            InvalidAuthenticationResponse, "Could not parse authentication credential"
         ):
             parse_authentication_credential_json(
                 {
@@ -265,7 +265,7 @@ def test_success_from_dict(self) -> None:
                 "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiSjlyUFpWWnFWODlUSW53bzV3cU11R3dlZjdET0pZRi1OVHlMQnhHV2pjZi16amFzOFRTUTlMbXI3em4wSmpkMTQyMU1sV0ItS2JYdEs5RW5sN19JM3ciLCJvcmlnaW4iOiJodHRwczovL3dlYmF1dGhuLmlvIiwiY3Jvc3NPcmlnaW4iOmZhbHNlfQ"
             ),
         )
-        self.assertEqual(parsed.response.user_handle, "bW1pbGxlcg")
+        self.assertEqual(parsed.response.user_handle, base64url_to_bytes("bW1pbGxlcg"))
         self.assertEqual(parsed.type, "public-key")
         self.assertEqual(parsed.authenticator_attachment, AuthenticatorAttachment.PLATFORM)
 
@@ -302,6 +302,6 @@ def test_success_from_str(self) -> None:
                 "eyJ0eXBlIjoid2ViYXV0aG4uZ2V0IiwiY2hhbGxlbmdlIjoiSjlyUFpWWnFWODlUSW53bzV3cU11R3dlZjdET0pZRi1OVHlMQnhHV2pjZi16amFzOFRTUTlMbXI3em4wSmpkMTQyMU1sV0ItS2JYdEs5RW5sN19JM3ciLCJvcmlnaW4iOiJodHRwczovL3dlYmF1dGhuLmlvIiwiY3Jvc3NPcmlnaW4iOmZhbHNlfQ"
             ),
         )
-        self.assertEqual(parsed.response.user_handle, "bW1pbGxlcg")
+        self.assertEqual(parsed.response.user_handle, base64url_to_bytes("bW1pbGxlcg"))
         self.assertEqual(parsed.type, "public-key")
         self.assertEqual(parsed.authenticator_attachment, AuthenticatorAttachment.PLATFORM)

tests/test_parse_registration_credential_json.py

@@ -165,7 +165,7 @@ def test_handles_bad_authenticator_attachment(self) -> None:
 
     def test_raises_on_non_base64url_raw_id(self) -> None:
         with self.assertRaisesRegex(
-            InvalidRegistrationResponse, "Unable to parse registration credential"
+            InvalidRegistrationResponse, "Could not parse registration credential"
         ):
             parse_registration_credential_json(
                 {
@@ -181,7 +181,7 @@ def test_raises_on_non_base64url_raw_id(self) -> None:
 
     def test_raises_on_non_base64url_attestation_object(self) -> None:
         with self.assertRaisesRegex(
-            InvalidRegistrationResponse, "Unable to parse registration credential"
+            InvalidRegistrationResponse, "Could not parse registration credential"
         ):
             parse_registration_credential_json(
                 {
@@ -197,7 +197,7 @@ def test_raises_on_non_base64url_attestation_object(self) -> None:
 
     def test_raises_on_non_base64url_client_data_json(self) -> None:
         with self.assertRaisesRegex(
-            InvalidRegistrationResponse, "Unable to parse registration credential"
+            InvalidRegistrationResponse, "Could not parse registration credential"
         ):
             parse_registration_credential_json(
                 {

webauthn/helpers/options_to_json.py

@@ -23,13 +23,10 @@ def options_to_json(
             _rp["id"] = options.rp.id
 
         _user: Dict[str, Any] = {
+            "id": bytes_to_base64url(options.user.id),
             "name": options.user.name,
             "displayName": options.user.display_name,
         }
-        if isinstance(options.user.id, bytes):
-            _user["id"] = bytes_to_base64url(options.user.id)
-        else:
-            _user["id"] = options.user.id
 
         reg_to_return: Dict[str, Any] = {
             "rp": _rp,

webauthn/helpers/parse_authentication_credential_json.py

@@ -24,7 +24,7 @@ def parse_authentication_credential_json(json_val: Union[str, dict]) -> Authenti
             raise InvalidJSONStructure("Unable to decode credential as JSON")
 
     if not isinstance(json_val, dict):
-        raise InvalidJSONStructure("Credential is not a JSON object")
+        raise InvalidJSONStructure("Credential was not a JSON object")
 
     cred_id = json_val.get("id")
     if not isinstance(cred_id, str):
@@ -58,19 +58,23 @@ def parse_authentication_credential_json(json_val: Union[str, dict]) -> Authenti
     except ValueError as cred_type_exc:
         raise InvalidJSONStructure("Credential had unexpected type") from cred_type_exc
 
-    # Pass on whatever we might have received back for `userHandle`, it's more important for the RP
-    # than response verification. This SHOULD be the same UTF-8 string specified as
-    # `user_id` when calling `generate_registration_options()`, unless something on the front end
-    # is acting up.
     response_user_handle = cred_response.get("userHandle")
+    if isinstance(response_user_handle, str):
+        # The `userHandle` string will most likely be base64url-encoded for ease of JSON
+        # transmission as per the L3 Draft spec:
+        # https://w3c.github.io/webauthn/#dictdef-authenticatorassertionresponsejson
+        response_user_handle = base64url_to_bytes(response_user_handle)
+    elif response_user_handle is not None:
+        # If it's not a string, and it's not None, then it's definitely not valid
+        raise InvalidJSONStructure("Credential response had unexpected userHandle")
 
     cred_authenticator_attachment = json_val.get("authenticatorAttachment")
     if isinstance(cred_authenticator_attachment, str):
         try:
             cred_authenticator_attachment = AuthenticatorAttachment(cred_authenticator_attachment)
         except ValueError as cred_attachment_exc:
             raise InvalidJSONStructure(
-                "Credential has unexpected authenticatorAttachment"
+                "Credential had unexpected authenticatorAttachment"
             ) from cred_attachment_exc
     else:
         cred_authenticator_attachment = None
@@ -90,7 +94,7 @@ def parse_authentication_credential_json(json_val: Union[str, dict]) -> Authenti
         )
     except Exception as exc:
         raise InvalidAuthenticationResponse(
-            "Unable to parse authentication credential from JSON data"
+            "Could not parse authentication credential from JSON data"
         ) from exc
 
     return authentication_credential

webauthn/helpers/parse_registration_credential_json.py

@@ -25,7 +25,7 @@ def parse_registration_credential_json(json_val: Union[str, dict]) -> Registrati
             raise InvalidJSONStructure("Unable to decode credential as JSON")
 
     if not isinstance(json_val, dict):
-        raise InvalidJSONStructure("Credential is not a JSON object")
+        raise InvalidJSONStructure("Credential was not a JSON object")
 
     cred_id = json_val.get("id")
     if not isinstance(cred_id, str):
@@ -72,7 +72,7 @@ def parse_registration_credential_json(json_val: Union[str, dict]) -> Registrati
             cred_authenticator_attachment = AuthenticatorAttachment(cred_authenticator_attachment)
         except ValueError as cred_attachment_exc:
             raise InvalidJSONStructure(
-                "Credential has unexpected authenticatorAttachment"
+                "Credential had unexpected authenticatorAttachment"
             ) from cred_attachment_exc
     else:
         cred_authenticator_attachment = None
@@ -91,7 +91,7 @@ def parse_registration_credential_json(json_val: Union[str, dict]) -> Registrati
         )
     except Exception as exc:
         raise InvalidRegistrationResponse(
-            "Unable to parse registration credential from JSON data"
+            "Could not parse registration credential from JSON data"
         ) from exc
 
     return registration_credential

webauthn/helpers/structs.py

@@ -187,7 +187,7 @@ class PublicKeyCredentialUserEntity:
     """Information about a user of a Relying Party.
 
     Attributes:
-        `id`: An "opaque byte sequence" that uniquely identifies a user. Typically something like a UUID, but never user-identifying like an email address. Cannot exceed 64 bytes.
+        `id`: An "opaque byte sequence" that uniquely identifies a user. Typically something like a UUID, but never user-identifying like an email address. Cannot exceed 64 bytes. These bytes should be stored internally alongside your normal user identifier and only used for WebAuthn.
         `name`: A value which a user can see to determine which account this credential is associated with. A username or email address is fine here.
         `display_name`: A user-friendly representation of a user, like a full name.
 

webauthn/registration/generate_registration_options.py

@@ -1,6 +1,6 @@
 from typing import List, Optional
 
-from webauthn.helpers import generate_challenge
+from webauthn.helpers import generate_challenge, generate_user_handle, byteslike_to_bytes
 from webauthn.helpers.cose import COSEAlgorithmIdentifier
 from webauthn.helpers.structs import (
     AttestationConveyancePreference,
@@ -43,8 +43,8 @@ def generate_registration_options(
     *,
     rp_id: str,
     rp_name: str,
-    user_id: str,
     user_name: str,
+    user_id: Optional[bytes] = None,
     user_display_name: Optional[str] = None,
     challenge: Optional[bytes] = None,
     timeout: int = 60000,
@@ -58,8 +58,8 @@ def generate_registration_options(
     Args:
         `rp_id`: A unique, constant identifier for this Relying Party.
         `rp_name`: A user-friendly, readable name for the Relying Party.
-        `user_id`: A unique identifier for the user. For privacy reasons it should NOT be something like an email address.
         `user_name`: A value that will help the user identify which account this credential is associated with. Can be an email address, etc...
+        (optional) `user_id`: A collection of random bytes that identify a user account. For privacy reasons it should NOT be something like an email address. Defaults to 64 random bytes.
         (optional) `user_display_name`: A user-friendly representation of their account. Can be a full name ,etc... Defaults to the value of `user_name`.
         (optional) `challenge`: A byte sequence for the authenticator to return back in its response. If no value is specified then a sequence of random bytes will be generated.
         (optional) `timeout`: How long in milliseconds the browser should give the user to choose an authenticator. This value is a *hint* and may be ignored by the browser.
@@ -78,12 +78,15 @@ def generate_registration_options(
     if not rp_name:
         raise ValueError("rp_name cannot be an empty string")
 
-    if not user_id:
-        raise ValueError("user_id cannot be an empty string")
-
     if not user_name:
         raise ValueError("user_name cannot be an empty string")
 
+    if user_id:
+        if not isinstance(user_id, bytes):
+            raise ValueError("user_id must be bytes")
+    else:
+        user_id = generate_user_handle()
+
     ########
     # Set defaults for required values
     ########
@@ -111,7 +114,7 @@ def generate_registration_options(
             id=rp_id,
         ),
         user=PublicKeyCredentialUserEntity(
-            id=user_id.encode("utf-8"),
+            id=user_id,
             name=user_name,
             display_name=user_display_name,
         ),