Prevent user from making multiple accidental charges.

Thanks for all the details. I'm thinking if there is anyway to reproduce this...

Okay thanks for the update.

I tested successful payments after a decline back while developing, but I didi't see any issues. If you are able, can you please check the stripe logs to see the steps that happened. If you can reproduce the problem then we can try to fix it.

Also, did the first customer actually pay 3 times in your stripe dashboard?

Good work on finding the problem! The patch looks good. I'll take a closer look and test it out a little later.

I'll get back to you for #8, I haven't had a chance to read it yet.

(I've updated the title to better reflect the problem.)

Reviewed patch....

1. Back button is removed, but submit button stays - in fact it looks like its duplicated multiple times. Not sure where this is coming from, could be something on our end - I'll investigate.

2. The spinner appears but doesn't rotate in Chrome

3. Its not good practice to hard set the text in the JS.
$( ".loader" ).after( '<div class="order-processing-message"><p>We are processing your order!</p></div>' );
Suggest making this translatable.

I am also seeing duplicate orders on regular non-recurring transactions. They aren't very often, however, I've had at least two out of maybe 30 - 40 transactions in the last 30 days that were duplicates and caused a refund. Stripes refund fees are $15 per refund...

If I get some time, I'll try to do some testing in my sandbox to see if double submission is the problem. Might take a bit before I have time though :-/

I was just looking at this issue last night. I'm planning to add the finishing touches to get it ready for the next release.

@AndraeRay I contacted stripe support suggesting they deal with duplicate transactions on their end as a hypothetical fix...

They responded with an API call that can be used on this end to help prevent the duplicate transactions. Since I have no idea how to figure out code, I don't know if it's already in what you have, but i'll leave here what they responded with. It might be something you aren't aware of:

Further to that I see that you appear to have a number of payments that are creating annoying duplicate charges upon a failed attempt generating on a percentage of your payments, of which there is definitely a means to counteract that our Engineers have made for Merchants such as yourself facing these problems.

This is called Idempotent Requests which essentially just means that the system will prevent duplicate transactions from being processed, the API supports idempotency for safely retrying requests without accidentally performing the same operation twice. This is useful when an API call is disrupted in transit and you do not receive a response. For example, if a request to create a charge does not respond due to a network connection error, you can retry the request with the same idempotency key to guarantee that no more than one charge is created:

https://stripe.com/docs/api/idempotent_requests

@philsward
Thanks for sharing that. I'll see how Idempotent Requests can be used to fix this.

@jas1988, good point. I'll keep that ticket in mind.

Strangely enough, I cant reproduce this problem. I followed the steps from #6 and I clicked submit order over 5 times, but only one charge was processed. I tried this a few times and even throttled my connection.

I am using Firefox. I'll try another browser.

Same with Chrome, I cannot reproduce this problem. No matter how many times I click submit, it only goes through just once.

Are you guys able to reproduce? Maybe stripe fixed it on their end?

I haven't had a chance to test, however looking at the two most recent duplicate transactions, everything about them is identical except the payment ID. They both use the exact same payment method ID and future payment session ID. Maybe those things don't matter in this discussion.

Could it be an apache configuration that is causing it? For example using FPM vs mod_php? Worker vs Event? PHP version? Dunno... Throwing out possibilities.

Occasionally I will get two emails for the exact same order. I haven't had a chance to see if there's any correlation between the duplicate emails and duplicate charges, however the most recent duplicate stripe charge did NOT come with a duplicate email.

Another thought is to add some logging into the Drupal side to watch every stripe transaction for the purpose of trying to track down the culprit. (a quick check didn't show anything for uc_stripe in the log filter area) If you implement this, make sure to set a check for whether it is installed and if it isn't, ignore trying to write the logs. UC back in the day used to freak out if watchdog (database logging) was completely uninstalled.

@philsward, Good point it might be a configuration difference. I'll try on my dev server as well.

We could add more logging, but i'm not sure how we would get valuable information from it. In this case we'll see 3 charges for multiple clicks but I'm not sure if it would help.

My thoughts are that If someone can still reproduce it, then we can test out the Idempotent Requests to verify that it fixes it.

Dropping this here:

http://www.antradar.com/blog-avoid-stripe-double-charges

and this:

https://news.ycombinator.com/item?id=22312172

Sorry for the long delay on this one. As suggested, the use of stripe’s Idempotency key seems to be the best approach.

I have also re-enabled retries from the confirm order page. Initially the customer could only go back and re-enter their information. It was set up that way because I never considered that network issues could be the reason for failures.

This fix means that if a customer is on the confirm page and there is a network error and they retry, a duplicate charge will not happen.
Also if they accidentally click submit more than once a duplicate charge will not happen.

Note: In the situation where there is a network error on the confirm page, and it gets approved on the Stripe end but not drupal end, if the customer clicks back and renters their information, a new idempotency key has to generated, so it won’t prevent duplicates that way. Hopefully this is a rare event though. This is because Stripe won’t accept the same idempotency key if payment or Intent information changes.

I'll set this to needs review incase anyone want's to test it out before I add it to dev.

I was able to do multiple clicks by adding this to the dev tools console on the checkout review page.

for (var i=0;i<10;i++){
  jQuery('#edit-submit').click();
}

Give me a bit, but I think I can probably get some decent feedback on this. Let me talk to my programmer buddy first before I switch what he's given me, over to testing your commits.

Passing along some thoughts from "the programmer"

Thought #1

In general the patch is good, but I do not feel that including the "payment method id" in the idempotency key is good, but it is just a feeling and I cannot explain why, or what would be better than that.
I mean this line:
$idempotency_key = _uc_stripe_create_idempotency_key($order_id . $amount . $payment_method_id);

Thought #2

To avoid problems when user clicks Back and reenters information, the Checkout Review form build ID could be used as idempotency key, instead of order ID, amount and payment method id.

The uc_stripe_form_uc_cart_checkout_review_form_alter() function could generate and save the idempotency key like this:
$_SESSION['stripe']['idempotency_key']=_uc_stripe_create_idempotency_key($form['#build_id']);

Then uc_stripe_renew() and _uc_stripe_confirm_payment() could use it like this:
$idempotency_key = $_SESSION['stripe']['idempotency_key'];

Here is a patch that combines the patch of #22 and the idea mentioned in #25

@philsward,

Thanks for submitting the patch, I reviewed the patch and found a situation where it causes duplicate payments. When on the checkout review page:
1. Click submit order
2. Quickly disable disable wifi before it succeeds.
3. Re-enable wifi, and submit order.

This situation simulates a network error, and is described here: #3075596: charge at stripe but no order at Drupal admin due to network failure, so no response

You will see two charges in your stripe dashboard for this situation.

Using the form build Id can be a challenge because it will change on each reload of that form, if it changes, Stripe would not know the requests are connected and should not process them multiple times.

Committed patch from #22
Marking as fixed.

Alright, first and foremost, sorry for having to change the status...

Can you take a look at the latest patch and see what you think?

Here's from the programmer:

AndraeRay, thank you for your review. Here is another patch suggestion that attempts to solve also your "Note" in #22:
"Note: In the situation where there is a network error on the confirm page, and it gets approved on the Stripe end but not drupal end, if the customer clicks back and renters their information, a new idempotency key has to generate, so it won’t prevent duplicates that way. Hopefully this is a rare event though."

I have seen this event happened several times, and the time between the two successful transactions was about 2 minutes, indicating that the customer used the Back button.
The attached patch attempts to generate an idempotency key based on the most important order values, so that if the customer uses the Back button after an error and pays again without changing the order, the same idempotency key is generated.
If you like the idea, but not the used order values, feel free to add or remove some.

I looked at your patch, and I think the changes you are suggesting will result in a stripe API error.

I initially created the idempotency key using only order_id and order_total.

In the situation where the user gets a network error, and clicks back and re-enters the same information it will use the same idempotency key but with a new Stripe PaymentMethodId. The stripe API will throw an error if you use the same idepondency key with diff params like paymentMethodId.

Fixing the problem I described would need to guarantee that the paymentId didn’t change, but because we have to cater to users without accounts it makes it more challenging.

Either way, since users can retry their order after a network failure, you may not see any more of those duplicates of 2 mins part.

If you would to see more changes It will be less confusing if you create a new ticket since this has already been committed to the codebase.

Setting this back to fixed.

I'm using that code on my site, so I guess if there's errors, I'll know one way or the other.

Thanks for looking into it.