search

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.6k stars 134 forks source link

TypeError: Cannot read properties of undefined (reading 'map') #196

Closed Spomky closed 2 years ago

Spomky commented 2 years ago

Hi,

I have an error because of missing excludeCredentials property at startRegistration.ts#L34. It seems that this line of code expect excludeCredentials to be present and to be a list, but it may be absent as it is not required by the PublicKeyCredentialCreationOptions definition.

In the absence of this property, should the library default it to an empty list []?

MasterKale commented 2 years ago

Yeah, an empty array is fine to set for excludeCredentials to fix the issue you're having.

Are you using generateRegistrationOptions() from the server library? If you are I'd like to see how you're calling it because it's supposed to set an empty array when its optional excludeCredentials argument is omitted. If it's not setting that empty array in the return value when I need to tighten that up since the front end library expects it to be set.

Spomky commented 2 years ago

Hi @MasterKale,

Yeah, an empty array is fine to set for excludeCredentials to fix the issue you're having.

This is the way I addressed the issue. The options was generated by this PHP file (it is now fixed).

Setting an empty array when missing seems more convenient to me than throwing an exception due to an undefined property. What do you think?

MasterKale commented 2 years ago

Setting an empty array when missing seems more convenient to me than throwing an exception due to an undefined property. What do you think?

The fact is @simplewebauthn/browser's methods were created to pair with output from @simplewebauthn/server's methods. The ability to use @simplewebauthn/browser's functionality with another server library is not something I explicitly support, but of course there's nothing preventing another library from conforming to startRegistration()'s API, for example, by returning PublicKeyCredentialCreationOptionsJSON which does require the existence of an excludeCredentials array.

(Fun fact, this is why Duo's py_webauthn library works so well with @simplewebauthn/browser: when I wrote the latest version of py_webauthn I intentionally made it easy to transform its generate_registration_options() output into a structure matching PublicKeyCredentialCreationOptionsJSON so I could use @simplewebauthn/browser for the front end 😛)

I won't be making any changes to startRegistration() at this moment in time. That said if you choose to update your server's output to match @simplewebauthn/browser's input expectations then I don't think you'll have any more issues like this.

Spomky commented 2 years ago

I won't be making any changes to startRegistration() at this moment in time.

OK now I clearly understand the context. I made the changes on server side so that's not an issue anymore. Regards.