Limit Stripe scripts and cookies to pages where they are required

Because according to the documentation:

To best leverage Stripe’s advanced fraud functionality, include this script on every page of your site, not just the checkout page. This allows Stripe to detect anomalous behavior that may be indicative of fraud as customers browse your website.

Thanks, torgosPizza.

I would be happy to make this configurable if Stripe can give us an indication that this is allowed. We just followed Stripe's recommendation here.

Definitely stripe JS is no requirement to be running through the entire Drupal.

With plain hook_page_attachments is loaded everywhere. No role / path restrictions.

Agree with @zenimagine to be restored to previous behavior as default one. People can opt-in with their implementations of hook_page_attachments to add it to more places

I also strongly agree with @zenimagine and @valic. After upgrading to the release that loads on every page load, my site now slows down significantly because of the connection to Stripe.

Not sure that mailing Stripe would benefit to get better solution :-)

Regarding security not correct statements.

1. 3D applies only to Europe.
2. If someone utilizing the Stripe Radar feature, it makes sense to load on more places.

We per example use other tools, not Radar.

You can now disable loading everywhere with your page_attachment, and adding trough form alter only payment method place.

But within commerce_stripe we could have on payment method configuration checkbox to turn it on on entire site, or just to have it on where Stripe is loaded

*correction, this setting should not be in payment gateway settings, but rather on separate config storage.

Otherwise, it would be needed to load payment methods on hook_page_attachments to get if is globally enabled or not.
And we don't want that :-)

I can add a patch if we know in which directions this JS thingy is going :-D

I agree with @zenimage too. Loading stripe.js everywhere lacks site performance and gdpr compatibility. For every business in Europe gdpr has to be an important topic by law, although it's sometimes quite annoying from developer/site builder view.
IMO it should be possible to turn this feature on or off.

Perhaps it's also possible to define the urls where stripe.js can be loaded. I imagine here a textarea where the URLs can written down, perhaps even with wildcard support (comparable with block visibility in Drupal)

For example:
if the commerce shop is reachable at mydomain.com/shop then the defined URL could be /shop/*.

Here is a patch, it just re-added libraries where it was before, without any hook_page_attachments implementation.

Don't see the point on working on some heavy logic where it should be or not, just where is required.
Each site is different, with different requirements.

If someone wish to have it on the entire site, they can add simple hook_page_attachments as it were.
This is more documentation part assume. Or how-to section for users who utilize Stripe Radar

/**
 * Implements hook_page_attachments().
 */
function my_module_page_attachments(array &$page) {
  $page['#attached']['library'][] = 'commerce_stripe/stripe';
}

or the can write logic for them based on routes or paths.

/**
 * Implements hook_page_attachments().
 */
function my_module_page_attachments(array &$page) {

 // Skip admin routes.
  if (Drupal::service('router.admin_context')->isAdminRoute()) {
    return;
  } 

  $page['#attached']['library'][] = 'commerce_stripe/stripe';
}

Setting back to "needs review" because the tests are marked as passing.

+1 for this. I think loading on every page should be opt in, and I think it it probably quite inappropriate to ever load on admin pages except when absolutely necessary.

@zenimagine: Did you test the patch? Did it work for you? If so, maybe it's time to mark this patch as RTBC?

I can also confirm the patch works; I've been using it in production for about a year now.

One thing still to consider is whether to add an opt-in to load on every page as per #26, but given the way the current behavior significantly slows down sites, it would be great to get this patch in and then open a separate issue for adding some UI element to load stripe.js on every page.

The patch works by itself (marking RTBC on that basis), but collides with other more critical patches I need to use, so I can't test for edge-effects with my install.

I agree, this is not required by Stripe, is a considerable performance regression for the vast majority of sites, and a problem for cookie compliance.

UI opt-in for V3 global script attachment could be a follow-up.

@zenimagine, try something like this in a module or theme, it's what I'm using:

/**
 * Implements hook_form_BASE_FORM_ID_alter().
 */
function MYMODULE_form_commerce_checkout_flow_alter(&$form, \Drupal\Core\form\FormStateInterface $form_state, $form_id) {
  // Attach the Stripe javascript only in checkout.
  // @see https://www.drupal.org/project/commerce_stripe/issues/3083393
  $form['#attached']['library'][] = 'commerce_stripe/stripe';
}

/**
 * Implements hook_page_attachments_alter().
 */
function MYMODULE_page_attachments_alter(array &$attachments) {
  // Remove the global Stripe javascript.
  // @see https://www.drupal.org/project/commerce_stripe/issues/3083393
  $libraries = &$attachments['#attached']['library'];
  if (($key = array_search('commerce_stripe/stripe', $libraries)) !== false) {
    unset($libraries[$key]);
  }
}

Wow. @zenimagine, I hear your frustration. I only just discovered the problem during our pre-launch testing, and I did offer you a solution that works without patching on production and is pretty straightforward to implement.

I think this module does have an issue with maintainership, nothing is getting committed in any reasonable timeframe, and maybe the maintainers need to coordinate better and offer a roadmap. If our charity continues to use Commerce Stripe (not at all guaranteed at present), I'll try to help move that along. Note I am not a maintainer.

I hear you. Your frustration is valid, but your outrage is not. Presumably you are charging your clients, and benefiting from the non-profit open-source work that has been done? One solution, assuming you do not want to learn how to develop and support this module, would be to help fund development of the fixes and features you want for this module. You can pass the cost on to your clients.

The module isn't crap. But it is an API module dealing with a 3rd-party service, so there is an ongoing maintenance burden that has been imposed by Stripe itself - the Stripe API continues to change, things like GDPR and 3DS authorization happen, and suddenly everyone using the module expects somebody else to contribute effort for free to support those changes - all so you can continue to profit?

Again, I hear you, but you need to dial back the outrage and consider what your actual contribution here is. The patch at #21 intially failed because no module tests were passing anyway. You marked that patch RTBC, so you obviously consider yourself able to test in some way. After that, it was marked "needs work" because tests failed, then "needs review" because tests were passing again, but you didn't bother to re-test, and nobody else marked it RTBC, so why would you expect anyone to commit the patch?

@valic: Thanks for the patch, committed!
@John Pitcairn: Thank you for your help with testing, and also really appreciate you stepping up!

Just a quick note here in case anybody comes across a similar issue. I have a Payment method of Credit card (Stripe) - that is only displayed on certain conditions. One of these is postcode (we only ship and take credit card payment to local customers). When the page first loads without Stripe being used - the stripe JS library is not added - as the payment method is not present. If the postcode is then updated - and the payment methods updated via AJAX - we get a "Stripe is not defined".

I guess this is perhaps a timing issue - but I'm going to revert to using a conditional library include as per the comment #35 - so the stripe library is always available through checkout - despite some customers not actually needing it...

https://www.drupal.org/files/issues/2022-01-25/stripe_error.PNG

This whole change came about after having to put the contact, shipping and payment info blocks all within the same Order info pane - leaving space to put the "stripe review" on its own with the review block under the review pane - this then allows us to process 2FA overlays successfully.

If the Stripe library is being added by the ajax call, then this core bug could be the problem: #1988968: Drupal.ajax does not guarantee that "add new JS file to page" commands have finished before calling said JS.

If it's not being added by the ajax call, it should be.