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

Fallback webcrypto to global #457

Closed sergiocarneiro closed 1 year ago

sergiocarneiro commented 1 year ago

Fallback to globalThis.crypto when node:crypto exists but crypto.webcrypto does not.

It also adds the ability to throw MissingWebCrypto when webcrypto is missing.

This change makes the getWebCrypto function more robust and able to work in more runtime setups (e.g. Cloudflare Workers with node_compat enabled).

MasterKale commented 1 year ago

This change makes the getWebCrypto function more robust and able to work in more runtime setups (e.g. Cloudflare Workers with node_compat enabled).

Hello @sergiocarneiro, I'm wondering what the exact problem is you were having with the current way of picking which WebCrypto to use. Are you trying to make globalThis.crypto the default the library tries to reach for Node's because it somehow causes issues with non-Node runtimes?

sergiocarneiro commented 1 year ago

I'm wondering what the exact problem is you were having with the current way of picking which WebCrypto to use.

Hello @MasterKale. In my Cloudflare Worker, I have Node polyfills enabled. I'm not sure if it's a bug from their platform or not, but I know that what ends up happening is an environment that:

So without this change I submitted, using functions such as getRegistrationOptions throws the error "Cannot read properties of undefined (reading 'getRandomValues')" — the method is available through globalThis.crypto.

Are you trying to make globalThis.crypto the default?

Yes, and only override it if crypto.webcrypto is defined, not if node:crypto can be imported.

CameronWhiteside commented 1 year ago

+1 to this issue--would love to use this in a Cloudflare worker, but without @sergiocarneiro's updates, I'm also fully blocked with the same "Cannot read properties of undefined (reading 'getRandomValues')" issue.

MasterKale commented 1 year ago

@sergiocarneiro Thank you for the contribution. I took a stab at solving this problem with #468, and I also managed to add test coverage around getWebCrypto() that should ensure support for runtimes like your CF worker w/Node compat. In particular this test should capture the nuance of the runtime issue you've been experiencing:

https://github.com/MasterKale/SimpleWebAuthn/pull/468/files#diff-6ac413926c71b2fbaa8af5cffcb57c492adf245dc0d0ec40ec6ba179c537debcR88-R119

Tests just passed over there so I'm going to merge and cut a release with this fix tomorrow.