passwordless-id / webauthn

Webauthn / passkeys helper library to make your life easier. Client side, server side and demo included.
https://webauthn.passwordless.id
MIT License
454 stars 53 forks source link

Minor discrepancy in 'discoverable' option default value of register() #68

Closed hjaber closed 3 months ago

hjaber commented 3 months ago

I want to express my appreciation for your ongoing work on this library. It's been incredibly helpful, and you've been very responsive. I'm sorry to keep bringing up minor issues, I am migrating to V2 and run into things.

I've noticed a small inconsistency between the documentation and the implementation of the 'discoverable' option in the register() function:

Current implementation in client.ts:

authenticatorSelection: {
    userVerification: options.userVerification,
    authenticatorAttachment: getAuthAttachment(options.hints),
    residentKey: options.discoverable,
    requireResidentKey: (options.discoverable === 'required')
}

Suggestion for alignment (if this matches the intended behavior):

authenticatorSelection: {
    userVerification: options.userVerification,
    authenticatorAttachment: getAuthAttachment(options.hints),
    residentKey: options.discoverable ?? 'preferred', // add default fallback
    requireResidentKey: (options.discoverable === 'required')
}

Alternatively, the documentation could be updated if the current implementation is intentional.

dagnelies commented 3 months ago

Intended. The default according to the native spec is preferred.

hjaber commented 3 months ago

I'm sorry, but just want confirm this because I have different behavior with the passkey ceremony when I set discoverable: "preferred" explicitly rather than omitting it and relying on it to default,

MDN

If omitted, residentKey defaults to "required" if requireResidentKey is true, otherwise the default value is "discouraged".

Based on the implementation, if discoverable is omitted, wouldn't it default to "discouraged"?

dagnelies commented 3 months ago

Reopening for verification, you may be right