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.4k stars 121 forks source link

Idea: Add new helper methods for MFA, PWL, and Usernameless registration and authentication #160

Closed MasterKale closed 1 year ago

MasterKale commented 2 years ago

SimpleWebAuthn already makes interacting with the WebAuthn library fairly painless, but can I take things one step further? The thought occurred to me that I might be able to offer a higher-level abstraction to the registration and authentication options generation and response verification methods. These abstractions would set combinations of options appropriate for use of WebAuthn for MFA, Passwordless, and Usernameless "modes", with minimal arguments to specify things like allowCredentials:

Registration

Authentication

Or maybe a new, optional mode argument on the existing methods that you specify as "mfa" | "pwl" | "unl"? That might actually be cleaner 🤔

In any case the idea is, there are certain options you must set if you want to, for example, leverage WebAuthn for Passwordless login. Without digging into the docs, though, you might not think to set them. I think there's a chance here to help devs even further by offering a quick and easy way to generate appropriate options and verify responses that will help ensure secure authentication for all use cases.

JayHelton commented 2 years ago

Before you ask, yes im subscribed to the repo when new issues are added. 😆

I like the idea of having abstractions between resident keys and non resident keys using a terminology that fits the UX. I am not confident in that abstraction being MFA / PWDL, or UNL. What is the different between PWDL versus MFA in terms of the webauthn impl details? Off the top of my head, I cant think of any. That might change my opinion though.

In general, i prefer method abstractions over parameterized abstractions, like the mode argument proposal. Then again, Im biased towards a "functional" approach over imperitive.

MaKleSoft commented 2 years ago

That sounds like a great idea! I think one of the main hurdles for many devs trying to adopt WebAuthn is understanding all the different use cases and how to implement them. Heck, I still haven't fully grasped how "passwordless" or "usernameless" are supposed to work in practice. I think descriptive APIs like the ones you're suggesting are a great way to guide developers towards the solution they're looking for and help avoiding many pitfalls and frustrations when adopting a new technology.

MaKleSoft commented 2 years ago

Oh, and I vote for the mode argument instead of a ton of different functions. Just my preference though.

MasterKale commented 2 years ago

I like the idea of having abstractions between resident keys and non resident keys using a terminology that fits the UX. I am not confident in that abstraction being MFA / PWDL, or UNL. What is the different between PWDL versus MFA in terms of the webauthn impl details? Off the top of my head, I cant think of any. That might change my opinion though.

One of the key differences between MFA and PWL modes is that a passwordless implementation of WebAuthn must require user verification for it to be true multi-factor authentication in one authenticator interaction. In MFA mode the user can simply pass a User Presence check since they've provided "something you know" with the username+password they entered beforehand.

If I went with the "mode" argument, would it make sense for that to overwrite options the dev might otherwise set that would conflict with flags the "mode" argument would enforce? In my mind it would, I'd just need to communicate that in the docs. But maybe that's too much "magic"?

MaKleSoft commented 2 years ago

If I went with the "mode" argument, would it make sense for that to overwrite options the dev might otherwise set that would conflict with flags the "mode" argument would enforce? In my mind it would, I'd just need to communicate that in the docs. But maybe that's too much "magic"?

I would not override, but throw an error or at least give a clear warning. You could probably even add type guards so such a mistake would show up in their editor right away (if they are using typescript).

MasterKale commented 2 years ago

I think I've identified a way forward for this feature:

Screen Shot 2021-09-21 at 10 17 45 AM

I'll set up TS to error out on invalid options, and then warn at runtime if invalid options are set for devs using the library in a JS project.

JayHelton commented 2 years ago

I like the idea of having abstractions between resident keys and non resident keys using a terminology that fits the UX. I am not confident in that abstraction being MFA / PWDL, or UNL. What is the different between PWDL versus MFA in terms of the webauthn impl details? Off the top of my head, I cant think of any. That might change my opinion though.

One of the key differences between MFA and PWL modes is that a passwordless implementation of WebAuthn must require user verification for it to be true multi-factor authentication in one authenticator interaction. In MFA mode the user can simply pass a User Presence check since they've provided "something you know" with the username+password they entered beforehand.

If I went with the "mode" argument, would it make sense for that to overwrite options the dev might otherwise set that would conflict with flags the "mode" argument would enforce? In my mind it would, I'd just need to communicate that in the docs. But maybe that's too much "magic"?

I get it, but i have found inconsistencies for when requiring UV isnt enforced on keys if the pin is not set up. I cant say if those issues still exist in browser + OS combos. You would want to make sure that the error returned from the server on the missing UV is really clear for users of the lib and how it relates to an MFA "mode".

In addition, having a "mode" parameter feels very similar to the parameters using in cryptographic algorithms. It feels a little weird to imply there are different modes of operation in the generation of webauthn options, as opposed to just different parameters being specified or enforced.

Im on board with what you think would be the best UX, but I would still thumbs down a mode parameter.

MasterKale commented 2 years ago

I've been mulling over some other options, putting them down here to maybe drive some conversation around this.

"Modifier" functions to wrap options and verification methods?

/**
 * `asPWL()` would override some options out of `generateRegistrationOptions()` to, for example,
 * require user verification even if the options passed into `generateRegistrationOptions()` didn't.
 * Maybe with some console warning output to help devs realize when their options aren't suitable
 * for the targeted use case?
 */
const pwlOpts = asPWL(generateRegistrationOptions({...}));
/**
 * I initially thought to wrap `verifyRegistrationResponse()` in `asPWL()` too, but after writing
 * it out for sake of example here it wouldn't make sense to do that. It'd need to wrap the options
 * going into `verifyRegistrationResponse()` instead to override values like
 * `requireUserVerification` to require it.
 */
const pwlVerification = asPWL(verifyRegistrationResponse({...})); // No good
const pwlVerification = verifyRegistrationResponse(asPWL({...})); // "Correct", but not "simple"

Setting a global "mode" via SettingsService?

Another idea might be to use SettingsService to set a global "mode" for all invocations of generate...Options() and verify...Response(). You'd call something like "SettingsService.setMode("pwl")" on server init and from there on options would be overwritten when the typical methods are called.

The downside to this is that there'd be no obvious way to support both PWL and Usernameless use, since setting the mode on the fly might lead to race conditions where verifications are unintentionally evaluated with different rules.


I'm not feeling very confident in either of these approaches, but maybe there are some things worth salvaging out of these?

MasterKale commented 2 years ago

I get it, but i have found inconsistencies for when requiring UV isnt enforced on keys if the pin is not set up. I cant say if those issues still exist in browser + OS combos. You would want to make sure that the error returned from the server on the missing UV is really clear for users of the lib and how it relates to an MFA "mode".

@JayHelton Coming back to this point, I wouldn't want to try and write anything to identify when verification failure due to UV being false was because a security key didn't have a PIN set. There's zero feedback to the browser from WebAuthn that that's the case, so the RP wouldn't have any ability to know either.

That makes this endeavor seem like a slippery slope to head down because WebAuthn is notorious for not revealing much about errors beyond when a user tries to re-register something in excludeCredentials, and a generic "operation was cancelled" kind of error that might also be because of a timeout. I have very little confidence I'd be able to write anything "simple" but also robust enough to infer reasons for failure within MFA/PWL/UNL contexts.

That is to say maybe an educational angle is the way to go for this problem. Perhaps it's time to revive your cookbooks idea for the docs, and maybe a cheatsheet/flow chart to help quickly determine appropriate options for generate...Options() and verify...Response() 🤔

markbarton commented 2 years ago

I would love a flow chart for the appropriate options. BTW your project is very well written and documented - I am learning a lot.

MasterKale commented 2 years ago

I created #171 as a draft of how I might offer up opinionated versions of the four primary registration and authentication methods. Let me know what you think, I'll be mulling these over for the next few days.

MasterKale commented 2 years ago

The WebAuthn ecosystem has gone in an unexpected direction since I originally created this issue, namely with the upcoming debut of "passkeys", or "multi-device credentials" backed up/synced between devices using a platform authenticator vendor's respective cloud, in the consumer space.

In light of this new aspect of the API I can't help but think of any effort to abstract WebAuthn further, even more than SimpleWebAuthn, as something that would be best implemented as an add-on to popular frameworks like Express, Next.js, etc...

That is, I see it more prudent to encourage the creation of, say, an express-passkey package that defines Express-compatible request handlers to generate and validate options using SimpleWebAuthn. That package would be responsible for controlling how SimpleWebAuthn gets called specifically for enabling support for passkeys (i.e. requiring user verification during registration and auth, leaving allowCredentials empty, etc...)

I, meanwhile, would continue to maintain SimpleWebAuthn as an abstraction of the WebAuthn spec itself with no other framework-specific abstractions to distract from my efforts to make the API simple to use. And in lieu of a PR like #171 I could instead augment the docs site with guidance on what options to set for a given mode (e.g. "mfa", "passwordless", and "passkeys") to help library maintainers identify what to set when they're building such integration libraries.

Yeah, that sounds like a good plan 🤔

MasterKale commented 2 years ago

I've closed out PR #171 but will leave this issue open so I remember to update the docs.

MasterKale commented 1 year ago

Superseded by #264