web-auth / webauthn-framework

FIDO-U2F / FIDO2 / Webauthn Framework
MIT License
397 stars 51 forks source link

Unable to create the response object when deserializing AttestationResponse #558

Closed joostdebruijn closed 5 months ago

joostdebruijn commented 5 months ago

Version(s) affected

4.8.0

Description

While working on getting around the deprecations in v4.8.0, I encountered a problem while denormalizing an AttestationResponse. The AuthenticatorResponseDenormalizer checks if the response doesn't contain a authenticatorData property at line 28. However, the responses I got have an authenticatorData property in the response. Therefore, I got an: InvalidDataException with the message Unable to create the response object.

How to reproduce

I got the following response from the browser (using simplewebauthn/browser v9.0.3) - it's from a virtual authenticator, but the response from a 'real' authenticator is the similar:

{
    "id": "mfT[...]Lb_oA",
    "rawId": "mfT[...]Lb_oA",
    "response": {
        "attestationObject": "o2NmbXRkbm9[...]LjRLRfJYhwBRQ",
        "clientDataJSON": "eyJ0eXBlIjoid2ViYXV0aG4u[...]OmZhbHNlfQ",
        "transports": [
            "hybrid",
            "internal"
        ],
        "publicKeyAlgorithm": -7,
        "publicKey": "MFkwEwYHK[...]Zd7dELjRLRfJYhwBRQ",
        "authenticatorData": "nni_YSTKRi1wlRw6[...]OIwjROo9RO63x8rvsZd7dELjRLRfJYhwBRQ"
    },
    "type": "public-key",
    "clientExtensionResults": {
        "credProps": {
            "rk": true
        }
    },
    "authenticatorAttachment": "cross-platform"
}

Then, I try to denormalize it:

$serializer = (new WebauthnSerializerFactory($attestation_manager))->create();
$pk_credential = $serializer->deserialize($request->getContent(), PublicKeyCredential::class, 'json');

Possible Solution

I think the check ! array_key_exists('authenticatorData', $data) should be removed.

Additional Context

User agent: Chrome 122.0.6261.58 PHP: 8.3.3

I'm not using the Symfony bundle, but I have an own implementation in a Laravel-application.

joostdebruijn commented 5 months ago

Fixed by merging #559.

github-actions[bot] commented 4 months ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.