mylofi / webauthn-local-client

Browser-only utils for locally managing WebAuthn (passkey) API
https://mylofi.github.io/webauthn-local-client/
MIT License
100 stars 3 forks source link

Encapsulate WebAuthn support checks in functions to allow SSR support #2

Closed tonydangblog closed 3 months ago

tonydangblog commented 3 months ago

Issue

Many frameworks (such as Next.js, Nuxt, SvelteKit, etc.) will run JS code server-side in order to server-side-render (SSR) html.

The webauthn-local-client library currently requires using the navigator object to check for WebAuthn support at the top level of the module. This means that if the library is imported with one of the above frameworks, an error will be thrown since navigator does not exist on the server.

Changes

Encapsulate the checks for WebAuthn support into functions that can be explicitly called on client-side only. Ex:

import { checkWebAuthnSupport, checkConditionalMediationSupport } from "webauthn-local-client";

// only call if on client-side
let supportsWebAuthn = await checkWebAuthnSupport();
let supportsConditionalMediation = await checkConditionalMediationSupport();
getify commented 3 months ago

@tonydangblog

Perhaps we can address this in a more simple way.

This line is the one in question, right?

navigator.credentials &&

So if we change that line to:

typeof navigator != "undefined" &&
typeof navigator.credentials != "undefined" &&

I think that makes it safe (in that the check will silently fail server-side instead of throwing). Thoughts?


Moreover, we have a similar issue with this line in external.js, right?

var script = document.createElement("script");

So I think we could address that by adding this line at the top of loadScript():

if (typeof document == "undefined") return;

WDYT?


But, before we do these fixes, I'd like to think carefully about what it implies for these SSR environments. If people include this library in pages that are SSR'd, are they relying on the "supports" detection to decide what they render from the server? There's no way to run those detections server-side obviously, so the server environment will always indicate no support.

I'm concerned whether these patterns around modern framework SSR have, or don't have (?), mechanisms in place to defer or handle such this-is-client-side-only-code in projects or not. Our library is certainly not the only one out there that makes assumptions about only running in a true browser environment.

Do these SSR frameworks not have some way to tell their server logic to skip over running JS code that's not meant to run on a server?

It feels to me like this problem is not our problem, but the server's problem. I'd rather not convolute our library to constantly chase down any issues with "doesn't run on a server" when none of this library is designed to ever run on a server.

Open to feedback here.

tonydangblog commented 3 months ago

Checking for typeof navigator != "undefined" is a much simpler solution. I like it!

I did not take a look to see if external.js would work yet with the proposed change.

I made some changes but it's WIP. Let's discuss more before proceeding.


Regarding why this script would run on the server...

Let's say an app has a login page that imports this library, an SSR framework would run the JS when it renders the HTML on the server. The purpose of rendering the HTML is so that the page potentially loads quicker on the client-side without any CLS. When running the script server-side, it would indicate no support for WebAuthn and silently fail, which is ok since the rendering of the HTML was what was wanted here.

Then, on the client-side, the app will run again in a hydration step. This is where the real check for WebAuthn support will be.


Regarding framework support for conditional SSR...

It varies between frameworks. Most frameworks have the option to opt-in/opt-out of SSR at varying degrees of granularity (SSR can be enabled/disabled at a site-level, page-level, or, more-rarely, at a component level). In order to support the most amount of frameworks and users out there, I think it would be good for the library to check the environment and act accordingly.

Additionally, even if all frameworks supported conditional SSR at the most granular level, I think it's helpful to the end user to not have to think about this problem themselves if it can be handled at the library level.


At the end of the day, I think that a library that does take SSR into account will be helpful to more people. Let me know what you think. 🙂