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.62k stars 137 forks source link

Better error type identification in the browser #357

Closed mhamann closed 1 year ago

mhamann commented 1 year ago

Describe the issue

When catching errors thrown by the browser API, it's desirable to identify what type of error occurred so that the user experience can be altered accordingly.

For example, did the user cancel the authentication? Was the browser unable to fulfill the request for some reason? Based on the error thrown, the user could be prompted to try again, or in some cases (e.g., browser autofill init), just ignore the error altogether.

This would also help cases where autofill and explicit startAuthentication() requests are used in parallel: one to help a user leverage an existing passkey even if they're not familiar with the terminology and another for users who know what they want immediately. The autofill request might be intentionally cancelled due to a more explicit request being initiated.

Currently, the library throws generic Errors, but it would be useful to customize those errors and set some sort of code property that can be programmatically relied upon to not change, even though english string messaging might. (e.g., code: E_CANCELLED or code: E_NO_INPUTS)

I am happy to PR changes in this regard if its agreed that this would be a helpful addition.

SimpleWebAuthn Libraries

$ npm list --depth=0 | grep @simplewebauthn
├── @simplewebauthn/browser@7.0.1
├── @simplewebauthn/typescript-types@7.0.0
MasterKale commented 1 year ago

Hmm, it's true that Errors are being returned, but it's their name properties that differentiate the errors. @simplewebauthn/browser keeps the error name for spec compliance, but overwrites the message when it feels confident it knows why the more general error was thrown.

This proposal could be interpreted as a request to introduce SimpleWebAuthn-specific error codes. Is that the direction you're thinking this would go?

mhamann commented 1 year ago

Thanks for the follow up. Yes, exactly--I was thinking these would be lib-specific error codes. Right now, I'm parsing the message in order to do some differentiation, but that will likely be brittle long-term.

MasterKale commented 1 year ago

I was poking around Error on MDN and learned about cause:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/cause

Perhaps I can use this here 🤔

mhamann commented 1 year ago

That seems workable. I also tested Opera (listed as not supporting Error.cause), and it seemed to work fine. I opened an issue with MDN to hopefully update or clarify the documentation.

MasterKale commented 1 year ago

So far what I'm thinking of doing is creating a custom error (when I don't pass through the WebAuthn error), and including the original error as cause. I need your opinion though. Do I:

  1. Throw a single "SimpleWebAuthnError" from both startRegistration() and startAuthentication(), differentiating registration or authentication errors by error code (e.g. "ERR_REG_FOO", "ERR_AUTH_BAR", etc...)
  2. Throw a single "SimpleWebAuthnError" from both startRegistration() and startAuthentication(), and error codes are more general (e.g. "ERR_FOO", "ERR_BAR", etc...) because the type is implied by the error handler wrapping either method
  3. Throw a "RegistrationError" from startRegistration(), and an "AuthenticationError" from startAuthentication(), and error codes are more general (e.g. "ERR_FOO", "ERR_BAR", etc...)

I'm between 1 and 2 right now, maybe preferring 2 because if any error messages are similar between the two then I can reuse error codes.

mhamann commented 1 year ago

I like 2 as well. The context is implied, which makes handling errors from either function similar.

MasterKale commented 1 year ago

@mhamann I'm working on #367 to finally address this. What do you think about the following error codes?

Now's the time to make suggestions if any of these seem off for whatever reason.

mhamann commented 1 year ago

This looks fantastic. I can't think of anything missing, and those seem nicely descriptive of what caused the error.

MasterKale commented 1 year ago

@mhamann I just published @simplewebauthn/browser@7.2.0 that should resolve this issue 🚀

mhamann commented 1 year ago

Thanks very much! We'll upgrade ASAP!