MasterKale / SimpleWebAuthn

WebAuthn, Simplified. A collection of TypeScript-first libraries for simpler WebAuthn integration. Supports modern browsers, Node, Deno, and more.
https://simplewebauthn.dev
MIT License
1.63k stars 137 forks source link

Error when excludeCredentials is missing in creationOptionsJSON #296

Closed darckking closed 2 years ago

darckking commented 2 years ago

I use the webauthn-lib in my server implementation and it doesn't return 'excludeCredentials' in registration options when there is no credentials to exclude. This leads to TypeError: Cannot read properties of undefined (reading 'map') which happens here

const publicKey: PublicKeyCredentialCreationOptions = {
    ...creationOptionsJSON,
    challenge: base64URLStringToBuffer(creationOptionsJSON.challenge),
    user: {
      ...creationOptionsJSON.user,
      id: utf8StringToBuffer(creationOptionsJSON.user.id),
    },
    excludeCredentials: creationOptionsJSON.excludeCredentials.map(toPublicKeyCredentialDescriptor),
  };

Imo, the behavior of webauthn-lib is correct because if you look at PublicKeyCredentialCreationOptions, you see that excludeCredentials is not required and defaults to empty array.

MasterKale commented 2 years ago

Imo, the behavior of webauthn-lib is correct because if you look at PublicKeyCredentialCreationOptions, you see that excludeCredentials is not required and defaults to empty array.

Getting pedantic for a second, the behavior of my pair of libraries in SimpleWebAuthn are correct because they've been built to assume use with each other. Any support for other libraries by @simplewebauthn/browser is because those other libraries (or the project using such libraries) chose to follow the API of @simplewebauthn/browser's methods and pass in the options its methods require. 🙂

You're right that the spec says that excludeCredentials is optional and defaults to an empty array when it's not provided in the call to navigator.credentials.create(). I made the conscious decision a long time ago to be more explicit about those values and return their defaults from @simplewebauthn/servers's generateRegistrationOptions() when those values are omitted.

After sleeping on this issue I'm deciding not to make any changes to @simplewebauthn/browser to address this issue. Users of webauthn-lib can choose to include excludeCredentials: [] when they pass options output to @simplewebauthn/browser's startRegistration() to satisfy its type requirements. Alternatively such RP devs can fork the library and maintain a custom copy that can handle the omission of excludeCredentials, or simply use another library that works better for them.

To that last point I believe GitHub's own webauthn-json might have an API that better aligns with the output from a server library like webauthn-lib

darckking commented 2 years ago

Thank you for the reply.