Automattic / woocommerce-payments

Accept payments via credit card. Manage transactions within WordPress.
https://wordpress.org/plugins/woocommerce-payments/
Other
175 stars 69 forks source link

WCPay Subscription Products with free trial periods create subscriptions in Stripe billing engine even when checkout fails due to AVS failures. #6251

Open Brianmitchtay opened 1 year ago

Brianmitchtay commented 1 year ago

Describe the bug

We've had a few users come to support with customers being charged 4 or 5 times for a single subscription, while only one subscription shows in their WC admin area. Often this sub has been cancelled and the customer continues to be charged.

In these cases the user has failed the checkout process several times initially, before successfully making their purchase. The result is that multiple subscriptions are started in the Stripe billing engine, and proceed to bill per the set schedule, but are not connected to any order in the merchant's WC store.

Another interesting facet of these erroneous subscriptions is that the payments did not collect a transaction fee of any kind.

To Reproduce

We have been able to replicate the behavior

  1. On a site with a live WCPay account, using the built-in subscription functionality, create a test subscription product that has a free trial period.
  2. Take the product with free trial to checkout and use the "Address Check Fails" card from Stripe here to simulate failing the AVS check at checkout. Fail the check several times.
  3. Complete checkout successfully (further testing shows this step doesn't appear to be necessary).
  4. View the account's subscriptions from the Stripe dashboard and note that there is one subscription for each time checkout was attempted, the initial failed attempt will have "Collection Paused" but others will not.
  5. Check the WC store's backend and note that there is no indication of any extra subscriptions

Actual behavior

Every time a checkout attempt fails, a subscription is still created in the Stripe billing engine. The merchant has no way to know this aside from viewing subsequent mystery renewal transactions as they are charged in future periods for the error subscriptions.

Screenshots

These subscriptions do not collect a transaction fee when taking payment later on. Screenshot taken on 2023-05-02 at 09 15 19 UTC@2x

Expected behavior

If a checkout attempt is unsuccessful, a subscription should not be created in the Stripe billing engine. If a subscription is created in error, the merchant needs a way to know and resolve the situation.

Desktop (please complete the following information):

Additional context

We've seen this now at least twice, initially in 6252065-zen and 6264864-zen

jessy-p commented 1 year ago

According to Pc2DNy-3z-p2, this falls in Helix @haszari. Tagging as a part of Gamma Porter duties PcreKM-yM-p2.

haszari commented 1 year ago

@thenbrent @james-allan – this sounds relatively serious, possibly critical since it's charging renewals inappropriately. James do you have capacity to investigate and confirm impact, propose next steps?

thenbrent commented 1 year ago

Agreed this has the potential to be a serious issue. Especially if it happens for any failure, not just AVS failures, which I'm going to assume is the case (or at least, any failure with an otherwise valid card/payment method where future payments can succeed once the payment method is saved). I am surprised an initial transaction can fail due to incorrect data, yet recurring transactions using that saved payment method then succeed. I guess details like AVS aren't being validated for those follow-up transactions.

@Brianmitchtay thanks for the great bug report here! It's made it much easier to understand the issue. I also agree with the expected behavior in the original issue as being the fix.

james-allan commented 1 year ago

I'll take a look into it. We create the subscriptions in the Stripe Billing engine when WC Subscriptions creates the subscription. @thenbrent you'll probably remember that subscriptions are created quite early in the checkout process - before payment is taken. To get around scenarios like this where payment fails, we lean on Stripe's payment_behavior property.

Stripe's own docs describe it like this:

Use default_incomplete to create Subscriptions with status=incomplete when the first invoice requires payment, otherwise start as active. Subscriptions transition to status=active when successfully confirming the payment intent on the first invoice. This allows simpler management of scenarios where additional user actions are needed to pay a subscription’s invoice. Such as failed payments, SCA regulation, or collecting a mandate for a bank debit payment method. If the payment intent is not confirmed within 23 hours subscriptions transition to status=incomplete_expired, which is a terminal state.

When WC Payments collects the payment for the order and gets the payment method ID from Stripe, we then assign the payment method to the subscription effectively activating it.

In essence the process is suppose to be:

  1. Create "draft" incomplete Stripe billing subscriptions to match when WC Subscriptions does.
  2. When payment is taken and the payment method ID is added to the parent order, we add that payment method to the subscription.
  3. We tell Stripe to mark their record of the subscription first payment as complete (as we've now taken the payment ourselves).
  4. Stripe should activate the subscription.

With that process in mind, if these customers are getting multiple subscriptions created due to failures, and those extra subscription's are actually charging the customer, there's a gap somewhere in the process.

As Stripe docs success, it should terminate 23 hours after creation if it never received a response. I'll need to dig into the code and see where the possible gap is.

james-allan commented 1 year ago

So I've been able to replicate the issue and it looks like it's caused by the subscription at is created in Stripe with a free trial and Stripe automatically creates the first invoice with $0 and marks it as paid immediately before we've even attempted payment.

There's essentially no need to set the payment method on the subscription order to activate it. 🤔 This is a difference between Stripe and WC Subscriptions. We collect a payment method even for free trials, where as Stripe just assume the customer's default method.

I'll have to look into possible solutions.

james-allan commented 1 year ago

I'll have to look into possible solutions.

One possible idea would be to do something like this:

  1. Avoid setting the trial end date when the subscription is created at Stripe initially.
    • setting the free trial causes Stripe to mark the initial invoice as paid before we've had a chance to check if payment was made on the parent order.
  2. Once we've collected a payment method, then we set the trial end date, assign the payment method and activate.

This appears to work for me locally, but it is a little odd to update the first payment date in a request to Stripe telling it what the subscription' payment method is.

Another idea would be to hook into the failure on the checkout and send a request to cancel any previously created subscriptions. I haven't looked into this, but it should be feasible. This is a little risky though as a fatal shutdown would still leave the subscription at Stripe active.

A 3rd possible solution would be to use Stripe's concept of "invoice items". These are essentially items purchased with the subscription at the time of signup. We use these when the subscription has a free trial but a signup fee for example. When there is a free trial we could add invoice items to the subscription creation to prevent Stripe from automatically marking it as paid. This is a little hacky, though. These invoice items are actually never used. After we collect a payment method on checkout, we mark this fake first invoice as paid_out_of_band (ie Stripe's way of saying we collected the payment for this invoice in another way). Messing with that is probably just confusing. In my testing, this invoice item cannot be even 10 cents because Stripe just zero out the invoice and pull the funds from the customer's reserve funds. 🤷 There's probably some threshold amount we'd need to set but that just further contributes to the hackiness of it all.

Screenshot 2023-05-18 at 5 08 14 pm

This 10 cents end up in customer account's "Invoice credit balance" :

Screenshot 2023-05-18 at 5 25 23 pm
thenbrent commented 1 year ago

Once we've collected a payment method, then we set the trial end date, assign the payment method and activate.

That sounds like a good solution to me.

This appears to work for me locally, but it is a little odd to update the first payment date in a request to Stripe telling it what the subscription' payment method is.

That's fair. I assume the trial end date could be done in a separate request to alleviate that, but the performance improvements of having it done in the same one seem worth the oddness. Perhaps the code could be refactored to be more generic and treat this action like activating the subscription instead of just setting payment method?

Another idea would be to hook into the failure on the checkout and send a request to cancel any previously created subscriptions. I haven't looked into this, but it should be feasible. This is a little risky though as a fatal shutdown would still leave the subscription at Stripe active.

Yeah that is a real risk and worth guarding against. Ideally we're not actually creating the activated subscription in a way that future payments will be collected until we have everything we need for that to happen (i.e. a working payment method). It seems like Stripe handles that a bit differently to how we expect, so it's reasonable IMO to workaround that with oddness like updating a date at the time of updating payment method.

melek commented 1 year ago

6337588-zen - User had three 'ghost' subscriptions created this way. They've already upgraded to Woo Subscriptions proper, so the impact was limited.

csmcneill commented 1 year ago

6428370-zen had nine additional subscriptions created.

jacoswan commented 1 year ago

6442494-zen had 2 additional subscriptions created.