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

Runtime errors in Safari 13.1 using `@simplewebauthn/browser` #405

Closed hwhmeikle closed 1 year ago

hwhmeikle commented 1 year ago

Describe the issue

In Safari 13.1, i'm getting the following error:

Unexpected token ';'. Expected an opening '(' before a method's parameter list.

I think this is due to the fact that the package is using public class fields. According to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Public_class_fields they didn't gain support in Safari until 14.1.

Reproduction Steps

  1. Go to a website that imports the @simplewebauthn/browser package e.g. demo.authsignal.com in Safari 13.1
  2. See error

Expected behavior

I don't expect any of the library to work as this version of Safari does not support WebAuthn, but ideally it would not error.

Code Samples + WebAuthn Options and Responses

I believe it's breaking on this line in my app:

const isSupported = await platformAuthenticatorIsAvailable();

Dependencies

SimpleWebAuthn Libraries

@simplewebauthn/browser@7.2.0

Additional context

image

If this is something I can fix on my end let me know!

MasterKale commented 1 year ago

Hello @hwhmeikle, can you provide a basic reproduction that will recreate the issue? I'm particularly interested in whether you're experiencing this with @simplewebauthn/browser in a SPA, or a basic HTML page using the UMD bundle.

hwhmeikle commented 1 year ago

@MasterKale In production its happening in a Next.js app (pages directory). But I can reproduce using create-react-app and this code https://codesandbox.io/s/festive-worker-g4zpjq-g4zpjq?file=/src/App.tsx

Using browserstack: Issue reproduction in browserstack

hwhmeikle commented 1 year ago

@MasterKale I switched to using the UMD es5 bundle and it resolved the errors. Unfortunately, it does impact our initial page load a noticeable amount as we're having to use it with the beforeInteractive strategy.

MasterKale commented 1 year ago

Hello @hwhmeikle, I don't want "use the UMD bundle" to be the solution here so rest assured I'm thinking about solutions targeting typical use of this library in SPA situations.

I've been thinking about possible ways to solve this. Maybe I target a lower ES level when I compile browser, or...I dunno, it's mostly a matter of finding time right now to dig deeper into this.

hwhmeikle commented 1 year ago

@MasterKale Let me know if you want any help finding a solution for this. Targeting a lower ES level sounds like a good option to me - keen to hear your thoughts.

MasterKale commented 1 year ago

Thanks for the ping @hwhmeikle, I'll try and take a look at this again this week. I can test older versions of Safari with BrowserStack at least so I should be able to figure out a path forward.

MasterKale commented 1 year ago

Two things I'm going to try:

  1. Downgrade the TypeScript target to "ES2021", maybe "ES2020" if that doesn't work, here: https://github.com/MasterKale/SimpleWebAuthn/blob/master/packages/browser/tsconfig.json#L4C16-L4C22
  2. Stick with the existing "ES2022" target but remove the code public class field here: https://github.com/MasterKale /SimpleWebAuthn/blob/master/packages/browser/src/helpers/webAuthnError.ts#L20

I'll let you know how it goes.

MasterKale commented 1 year ago

@hwhmeikle This is weird, I tried both https://codesandbox.io/s/festive-worker-g4zpjq-g4zpjq?file=/tsconfig.json and a new React + TS project (via npm create vite@latest and then updated tsconfig.json to look like the one you included in that code sandbox.io project) but I can't seem to recreate the issue in macOS 10.15 (Catalina) and Safari 13.1.2 using BrowserStack:

Code Sandbox (w/@simplewebauthn/browser@7.2.0)

Screenshot 2023-08-15 at 7 40 54 PM

Local React project (w/@simplewebauthn/browser@7.4.0)

Screenshot 2023-08-15 at 7 41 43 PM

tsconfig.json

{
  "compilerOptions": {
    "lib": ["ES2015", "DOM"],
    "esModuleInterop": true,
    "jsx": "react-jsx",
    "strict": true,
  },
  "include": ["src"],
  "references": [{ "path": "./tsconfig.node.json" }]
}

App.tsx

import { useEffect, useState } from 'react'
import { platformAuthenticatorIsAvailable } from '@simplewebauthn/browser';

import './App.css'

function App() {
  const {
    isPasskeysSupported,
    isCheckingPasskeysSupport
  } = useDoesBrowserSupportPasskeys();

  if (isCheckingPasskeysSupport) {
    return null;
  }

  return (
    <div style={{ color: "black" }}>
      {isPasskeysSupported ? "passkeys supported" : "not supported"}
    </div>
  );
}

export default App

function useDoesBrowserSupportPasskeys() {
  const [isPasskeysSupported, setIsPasskeysSupported] = useState(true);
  const [isCheckingPasskeysSupport, setIsCheckingPasskeysSupport] = useState(
    true
  );

  useEffect(() => {
    const checkSupport = async () => {
      setIsCheckingPasskeysSupport(true);

      const isSupported = await platformAuthenticatorIsAvailable();

      setIsPasskeysSupported(isSupported);

      setIsCheckingPasskeysSupport(false);
    };

    checkSupport();
  }, []);

  return { isPasskeysSupported, isCheckingPasskeysSupport };
}

It's all working fine 🙃

hwhmeikle commented 1 year ago

@MasterKale I just tried with a vite/ts set up and it also worked for me, but then tried with both a create react app project and next pages directory project and got the error 😢 .

Fresh Next pages project generated with npx create-next-app@latest (using typescript): Screenshot of error

MasterKale commented 1 year ago

@hwhmeikle I did some testing this morning with npx create-next-app@latest as you suggested. I discovered that dropping the target down to ES2021 (Option 1 I proposed a couple weeks ago) fixes the issue:

Screenshot 2023-08-23 at 9 34 48 AM

I'm preparing a PR now and will cut a release today with this fix.

MasterKale commented 1 year ago

@hwhmeikle This issue should be resolved with the newly published @simplewebauthn/browser@8.0.2 ✌️

hwhmeikle commented 1 year ago

TYSM!