Prevent an attempt to use an expired card

Created on 21 June 2023, almost 2 years ago
Updated 7 June 2024, 10 months ago

Problem/Motivation

This is a sigificant bug because it can lead to customers being charged, although their sub has been cancelled and cannot be uncancelled.

Steps to reproduce

1. Create subscription.
2. Allow the payment method card to expire before renewal date.
3. Wait for the renewal to trigger.
4. At this point a payment intent is created for the expired card.
5. Wait until any grace period or dunning expires, and let the subscription cancel for failed payment.
6. Add a new, valid card.
7. Stripe takes payment on the basis of the earlier intent, but now the subsciption has been cancelled and is not reactivated.

Proposed resolution

Preventing any attempt to renew with an expired card would stop that possible squence of events.

It would also prevent the problems which arise when customers leave an expired payment method on the sytem in addition to a new payment method.

Implementing 'smarter saved card' would reduce the risk.

Additionally, allow a cancelled sub to be un-cancelled. I now have to work out how to un-cancel a sub where this happened, presumably using direct SQL commands, which is not ideal.

Remaining tasks

User interface changes

API changes

Data model changes

🐛 Bug report
Status

Closed: works as designed

Component

Code

Created by

🇬🇧United Kingdom John_B London (UK), Worthing (UK), Innsbruck (Tirol)

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @John_B
  • 🇬🇧United Kingdom John_B London (UK), Worthing (UK), Innsbruck (Tirol)
  • 🇬🇧United Kingdom John_B London (UK), Worthing (UK), Innsbruck (Tirol)
  • 🇳🇿New Zealand john pitcairn

    Shouldn't a fair bit of this be the responsibility of commerce_recurring?

  • Status changed to Closed: works as designed 10 months ago
  • 🇬🇧United Kingdom jonathanshaw Stroud, UK

    I'm not sure I believe point 7.

    I don't see how Stripe takes payment just because a customer card is updated on Drupal.

    If commerce_recurring tries to process a payment for an order for a cancelled subscription, that's an issue with commerce_recurring not commerce_stripe. If I remember right, by default commerce_recurring does exactly this, because technically the order has already fallen due and some sites might want to collect the money in this case; canncelling the sub only spares the customer from accruing future liabilities, not paying existing liabilities.

    I use this code to deal with this issue in my project:

    <?php
    
    namespace Drupal\my_project\EventSubscriber;
    
    use Drupal\commerce_order\Entity\OrderInterface;
    use Drupal\commerce_recurring\Event\SubscriptionEvent;
    use Drupal\Core\Entity\EntityTypeManagerInterface;
    use Drupal\state_machine\Event\WorkflowTransitionEvent;
    use Symfony\Component\EventDispatcher\EventSubscriberInterface;
    
    /**
     * Class RecurringSubscriptionChangeSubscriber. Provides subscriber for orders.
     */
    class RecurringSubscriptionChangeSubscriber implements EventSubscriberInterface {
    
      /**
       * The order storage.
       *
       * @var \Drupal\Core\Entity\EntityStorageInterface
       */
      protected $orderStorage;
    
      /**
       * The log storage.
       *
       * @var \Drupal\commerce_log\LogStorageInterface
       */
      protected $logStorage;
    
      /**
       * Constructs a new PaymentEventSubscriber object.
       *
       * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
       *   The entity type manager.
       *
       * @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException
       */
      public function __construct(EntityTypeManagerInterface $entity_type_manager) {
        $this->orderStorage = $entity_type_manager->getStorage('commerce_order');
        $this->logStorage = $entity_type_manager->getStorage('commerce_log');
      }
    
      /**
       * {@inheritdoc}
       */
      public static function getSubscribedEvents() {
        $events['commerce_subscription.cancel.post_transition'] = 'onCancel';
        $events['commerce_recurring.commerce_subscription.update'] = 'onUpdate';
        return $events;
      }
    
      /**
       * Cancel in-progress orders when a subscription is cancelled.
       *
       * OOTB commerce_recurring cancels drafts but not placed orders.
       *
       * @param \Drupal\state_machine\Event\WorkflowTransitionEvent $event
       *   The event.
       */
      public function onCancel(WorkflowTransitionEvent $event) {
        $subscription = $event->getEntity();
        $order_ids = $subscription->getOrderIds();
        if (empty($order_ids)) {
          return;
        }
    
        // Load outstanding orders and cancel them.
        $order_ids = $this->orderStorage->getQuery()
          ->condition('type', 'recurring')
          ->condition('order_id', $order_ids, 'IN')
          ->condition('state', 'needs_payment')
          ->accessCheck(FALSE)
          ->execute();
        $orders = $this->orderStorage->loadMultiple($order_ids);
        foreach ($orders as $order) {
          $order->getState()->applyTransitionById('cancel');
          ;
          $order->save();
          $this->logStorage->generate($order, 'my_project_order_subscription_canceled', [])->save();
        }
      }
    
      /**
       * Update orders when a subscription is updated.
       *
       * OOTB commerce_recurring makes limited changes to draft but
       * not placed orders. For the limited aspect see
       * https://www.drupal.org/project/commerce_recurring/issues/3200723
       * Not touching placed orders is by design; the subscription scheduled
       * changes field is designed to respect the fact that typically
       * subscribers cannot change a bill once it's fallen due.
       *
       * @param \Drupal\state_machine\Event\WorkflowTransitionEvent $event
       *   The event.
       */
      public function onUpdate(SubscriptionEvent $event) {
        $subscription = $event->getSubscription();
        $order_ids = $subscription->getOrderIds();
        if (empty($order_ids)) {
          return;
        }
    
        // Load outstanding orders and refresh them.
        $order_ids = $this->orderStorage->getQuery()
          ->condition('type', 'recurring')
          ->condition('order_id', $order_ids, 'IN')
          ->condition('state', ['draft', 'needs_payment'], 'IN')
          ->accessCheck(FALSE)
          ->execute();
        $orders = $this->orderStorage->loadMultiple($order_ids);
        foreach ($orders as $order) {
          $order->setRefreshState(OrderInterface::REFRESH_ON_SAVE);
          $order->save();
        }
      }
    
    }
    
  • 🇬🇧United Kingdom John_B London (UK), Worthing (UK), Innsbruck (Tirol)

    @jonathanshaw Thanks for sharing your comment and code!

Production build 0.71.5 2024