The check for browser auto fill doesn't take into account shadow DOMs (ie: web components)

treeder commented 3 months ago

The check here doesn't dive into the shadow DOM of web components so it won't see if there's an autocomplete input in a web component.

https://github.com/MasterKale/SimpleWebAuthn/blob/a4a178cc8daa4eb5106b9708456195f3aff9643f/packages/browser/src/methods/startAuthentication.ts#L59

While this is supported in the major browsers now: https://issues.chromium.org/issues/40251770

MasterKale commented 3 months ago

Hello @treeder, it's unclear to me from that Chromium bug report what I might need to be doing differently here to support inputs within web components. I'm simply calling a DOM API, isn't it the browser's responsibility to make sure the shadow DOM gets checked too?

treeder commented 3 months ago

Shadow doms aren't exposed, that's kind of the point of them, they are isolated from everything else. I'm not sure of the answer to this, but I can play around next week and try to figure it out. Maybe the solution is to just not do this explicit check. 🤷‍♂️

MasterKale commented 2 months ago

The best solution I've been able to brainstorm so far is to add an option to startAuthentication() to ignore the DOM check so it won't error out 🤔

treeder commented 2 months ago

Ya, that's what I said in my previous comment. If that check isn't really essential then being able to bypass would be great.

MasterKale commented 2 months ago

If that check isn't really essential then being able to bypass would be great.

Well, the check is accurate as per the working draft of the L3 spec that introduces conditional mediation:

If options.mediation is conditional and the user interacts with an input or textarea form control with an autocomplete attribute whose non-autofill credential type is "webauthn",

Note: The "webauthn" autofill detail token must appear immediately after the last autofill detail token of type "Normal" or "Contact". For example:

"username webauthn"

"current-password webauthn"

So I'm inclined to keep the check in. I'm also at the point where if I add any more arguments to startAuthentication() then I'll switch from positional options to an options object. That'd necessarily break the method's API, which means a major version release.

(Just leaving some notes as I triage open issues.)

treeder commented 1 month ago

Isn't that spec for what the browser should do, not what your library should do?

Not sure if this is useful, but here's an example from that chrome issue: https://nopwd.rocks/

MasterKale commented 1 month ago

Repro HTML file:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Shadow DOM attempt</title>
</head>
<body>
  <script>
    class SWANAutofill extends HTMLElement {
      shadowRoot = this.attachShadow({ mode: "open" });
      // shadowRoot = this;

      connectedCallback() {
        this.shadowRoot.innerHTML = `
          <label for="username">Username</label>
          <input
            type="text"
            name="username"
            autocomplete="username webauthn"
            autofocus
          />
        `;
      }
    }

    customElements.define('swan-autofill', SWANAutofill);
  </script>
  <h1>SimpleWebAuthn Autofill</h1>
  <h2>Shadow DOM Text</h2>
  <swan-autofill></swan-autofill>
  <script>
    // Check for an <input> with "webauthn" in its `autocomplete` attribute
    const eligibleInputs = document.querySelectorAll(
      "input[autocomplete$='webauthn']",
    );

    // WebAuthn autofill requires at least one valid input
    if (eligibleInputs.length < 1) {
      console.warn(
        'No <input> with "webauthn" as the only or last value in its `autocomplete` attribute was detected. Passkey autofill may not work as expected',
      );
    }

    // Start autofill
    navigator.credentials.get({
      mediation: 'conditional',
      publicKey: {
        challenge: new Uint8Array([1,2,3,4]),
      },
    });
  </script>
</body>
</html>

MasterKale commented 1 month ago

@treeder Check out the new @simplewebauthn/browser@11.0.0 - startAuthentication() gains a new verifyBrowserAutofillInput option that can be set to false to hopefully address your issue.

Just a heads up, the API of this method (and startRegistration()) changed to accept a single argument object, with the positional arguments as properties within. See the CHANGELOG for more info about this:

https://github.com/MasterKale/SimpleWebAuthn/blob/master/CHANGELOG.md#browser-positional-arguments-in-startregistration-and-startauthentication-have-been-replaced-by-a-single-object

Edit: And thanks for your patience while I got this feature out the door 🙏

treeder commented 1 month ago

I upgraded and looks like it works!

Here it is in action: https://thingster.app/login

Thanks!

MasterKale / SimpleWebAuthn

The check for browser auto fill doesn't take into account shadow DOMs (ie: web components) #597