webauthn-open-source / fido2-lib

A node.js library for performing FIDO 2.0 / WebAuthn server functionality
https://webauthn.io
MIT License
407 stars 120 forks source link

Allow nullable userHandle as stated in specs #113

Closed PathToLife closed 2 years ago

PathToLife commented 2 years ago

Hi, I'm creating an issue here documenting the mismatch in specs and then making a quick PR.

What

I get the error when running assertionResult(response): TypeError: expected 'response.userHandle' to be base64 String, ArrayBuffer, or undefined

response.userHandle is set to null

I modified the validateAssertionResponse function to validate the type internally

throw new TypeError(`expected 'response.userHandle' to be base64 String, ArrayBuffer, or undefined got ${req.response.userHandle}`);

which returns: ... got null

So I'm wondering if we should allow null? Any foreseen consequences?

Spec

According to the level 3 and level 2 docs, userHandle nullable, and should be accepted? https://w3c.github.io/webauthn/#dom-authenticatorassertionresponse-userhandle https://www.w3.org/TR/webauthn-2/#dom-authenticatorassertionresponse-userhandle docs-nullable

Background

I'm using import { get } from '@github/webauthn-json/browser-ponyfill' to sign the decoded PublicKeyCredentialRequestOptions from the server. This PublicKeyCredentialRequestOptions contains a list of allowCredentials (~3), some of which do not correspond to resident keys.

The userHandle is only set if I sign using a residentKey, if not, I get the TypeError on the server side. It may be good to allow null, if we agree this is applicable to spec.

Thanks!

JamesCullum commented 2 years ago

Hmm for me undefined matches the spec closely enough, but I agree, that there are small nuances. Thanks for the issue and PR 👍