SimpleWebAuthn icon indicating copy to clipboard operation
SimpleWebAuthn copied to clipboard

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

Open treeder opened this issue 1 year ago • 4 comments

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

treeder avatar Jul 25 '24 18:07 treeder

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?

MasterKale avatar Jul 26 '24 02:07 MasterKale

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. 🤷‍♂️

treeder avatar Jul 27 '24 18:07 treeder

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 🤔

MasterKale avatar Aug 26 '24 18:08 MasterKale

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.

treeder avatar Aug 26 '24 19:08 treeder

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.)

MasterKale avatar Sep 22 '24 04:09 MasterKale

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/

treeder avatar Sep 24 '24 13:09 treeder

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 avatar Oct 02 '24 14:10 MasterKale

@treeder Check out the new @simplewebauthn/[email protected] - 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 🙏

MasterKale avatar Oct 13 '24 16:10 MasterKale

I upgraded and looks like it works!

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

Thanks!

treeder avatar Oct 17 '24 16:10 treeder