stripe / stripe-js

Loading wrapper for Stripe.js
https://stripe.com/docs/js
MIT License
626 stars 154 forks source link

[BUG]: 3D Secure Authentication behind top layer elements #599

Open tinleym opened 6 months ago

tinleym commented 6 months ago

What happened?

3D Secure Authentication modals rely on a high z-index, which regardless of number is trumped by top layer elements (e.g. a dialog opened with .showModal()). This can make the modal inaccessible (if, for example, it's called from a dialog element opened with showModal).

https://developer.mozilla.org/en-US/docs/Glossary/Top_layer

Environment

No response

Reproduction

No response

brendanm-stripe commented 6 months ago

Can you share a basic reproduction of this to illustrate? What would you expect to happen vs what you observe?

tinleym commented 6 months ago

Here's a codesandbox: https://codesandbox.io/p/devbox/goofy-mountain-wwzgsc?file=%2Fapp%2F_lib%2Fmain.tsx%3A37%2C16

The card number in the dialog will trigger 3d secure. I'd expect the 3d secure modal to appear over the form modal, but it's behind, because top layer is above any z-index.

brendanm-stripe commented 6 months ago

Thanks for sharing that repro -- we're investigating this.

benkingcode commented 5 months ago

Also a big issue for us! Our checkout flow is in a native dialog.showModal() and 3D Secure totally breaks...

stale[bot] commented 5 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

tinleym commented 5 months ago

Some further activity

wesbos commented 4 months ago

Hitting this issue as well. I have a checkout in a dialog:

<dialog><StripePaymentElement/><dialog>

When using .showModal() on the dialog, the browser add its to the "Top Layer", which cannot be overwritten with z-index.

Because of this, the 3d secure popup is under my dialog. Paypal has the same issue with their embedded login.

The fix is for stripe to move their modals to a modally shown

element.

Right now for paypal I am closing my <dialog>, but I'm not finding an event for the PaymentElement

Screenshot 2024-06-13 at 12 54 41 PM

I've put my modal at 20% opacity here to show the issue:

Screenshot 2024-06-13 at 12 55 33 PM
wesbos commented 4 months ago

@tinleym @benkingcode - did you find any workaround to this? I'm trying to find a good way to detect if the 3DS modal is open so I can close mine. Doesn't look like Stripe offers an even for this, and detecting the iframe being inejcted seems tricky as there are no classes or src that I can grab onto..

tinleym commented 4 months ago

@wesbos I'm setting the dialog display to block, and then on submit I call dialog.close (it stays visible because of display: block, but is removed from the top layer), I then await my stripe calls, and then I call dialog.showModal to get the dialog back to its presubmission state.

In my case, I want the dialog visible and inert during a submission, whether 3DS is called or not, so that's as far as I got.

wesbos commented 4 months ago

ohhhh interesting trick - display block on the closed dialog. thank you - might help me out

benkingcode commented 3 months ago

@brendanm-stripe any update?

Neophen commented 2 months ago

This is going to become more and more of an issue as more and more people will start using dialog elements. We're running into this issue now also

brendanm-stripe commented 2 months ago

We have no updates to share about this yet.

An interim alternative Stripe payment flow would be generating confirmation tokens to confirm on your server, specifying use_stripe_sdk=true (ref) for handling actions, then if required you can call handleNextAction (ref) to display the 3ds flow with your modal/dialog dismissed/hidden (possible using an approach like this).

Lutymane commented 2 months ago

I faced the same issue. My current work around is to remove Stripe frame from its original wrapper <div> and place it in its own top <dialog> and then use that old <div> to clean up the <dialog>.

Really this behavior should be default in Stripe itself already, since showModal() is a baseline feature and easy to fall back with existing logic.

{
  /**
   * [wrap, dialog]
   */
  const map = new WeakMap<HTMLElement, HTMLDialogElement>();

  const mo = new MutationObserver(mutations => {
    for (let { addedNodes, removedNodes } of mutations) {
      for (let addedNode of addedNodes) {
        // @note node should be element
        if (!(addedNode instanceof HTMLElement)) continue;

        // @note element is not a dialog (fired when we insert dialog with stripe frame)
        if (addedNode.tagName === "DIALOG") continue;

        let stripeFrame = addedNode.querySelector("[name^=__privateStripeFrame]");

        if (stripeFrame) {
          const stripeDialog = document.createElement("dialog");

          stripeDialog.style.width = "100%";
          stripeDialog.style.height = "100%";

          // @note keep its original div wrapper for dialog clean up reasons
          map.set(stripeFrame.parentElement!, stripeDialog);

          // @note append stripe frame itself to our new dialog and then we
          stripeDialog.append(stripeFrame);
          document.body.append(stripeDialog);

          stripeDialog.showModal();
        }
      }

      // @note stripe.js::this.unmount removes wrapper element from <body> so we can track that too
      for (let removedNode of removedNodes) {
        const dialog = map.get(removedNode as HTMLElement);

        if (!dialog) continue;

        dialog.remove();
      }
    }
  });

  mo.observe(document.body, { childList: true });
}
wesbos commented 2 months ago

The fix via stripe support was to move the generation of tokens to the server, but I was too far in to change my solutions, so I made this brutal hook to check for the modal. Hopefully this is fixed soon

/**
 * Check if the Stripe modal is open.
 * @link https://github.com/stripe/stripe-js/issues/599
 * Stripe has no way to tell if their modal is open, and when using a dialog .showmodal(), it opens overtop of the Stripe modal.
 * This hook checks if the Stripe modal is open
 */
export function useStripeModal() {
  const [open, setOpen] = useState(false);
  const [checkoutNeedsToBeReopened, setCheckoutNeedsToBeReopened] = useState(false);
  const dialog = document.querySelector<HTMLDialogElement>('dialog.checkout');
  // Continually check if there is a div with the class of .StripeElement
  useInterval(() => {
    const stripeModal = document.querySelector(
      'div[tabindex="-1"] > iframe[name*="__privateStripeFrame"]'
    );
    setOpen(!!stripeModal);
  }, 250);

  useEffect(() => {
    console.log('Stripe modal status changed', open);
    // If the dialog is open and the stripe modal is open, close the dialog
    if(dialog?.open && open) {
      dialog.close();
      setCheckoutNeedsToBeReopened(true);
    } else if (checkoutNeedsToBeReopened && !open) {
      // If the dialog was closed and the stripe modal is closed, reopen the dialog
      dialog?.showModal();
    }
    // If the stripe modal is closed
  }, [open])
  return open;
}
ohnotnow commented 3 weeks ago

Somewhat tangential - but possibly of 👀 for @wesbos . Caleb Porzio was recently talking about the weirdness with the top level 'dialog' layer vs. z-index vs. manual vs. ..... stuff for his new component library.

https://notesonwork.transistor.fm/episodes/modals-and-popover-woes

(Not an affiliate link, blah)

Magellol commented 2 weeks ago

We also use the dialog element and ultimately faced the same issue with the 3D secure frame rendering below our modal that sits on the top layer.

I implemented something similar as shared by @Lutymane. It works well but I'm a bit worried about it relying on some implementation details from this library. For instance if the __privateStripeFrame name changes, it could break all of a sudden. I guess we need to be extra careful upgrading this lib since we rely on internals.

If we can't solve the z-index issue right away, it would be great if stripe would let us know when such frames like 3D secures are being displayed so we don't have to rely on the lib internals.

Note: this fix also works for stripe captcha challenges. It uses the same mechanism.