Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition causing duplicate orders with double-clicks on Braintree "Pay" button #10767

Closed
RJacksonm1 opened this issue Sep 4, 2017 · 8 comments

Comments

@RJacksonm1
Copy link

commented Sep 4, 2017

Preconditions

  1. Enable "Braintree" payment method
  2. Ability to process requests concurrently (in our case, Magento deployed across various servers)

Steps to reproduce

  1. Add items to basket and proceed through checkout, until its time to choose payment method.
  2. Select Credit Card payments (Braintree)
  3. Enter payment details (I'm using test credentials on a sandbox environment)
  4. Continually click the "Pay" button (There is a brief moment before a "Loading" interstitial appears, where the "Pay" button can be clicked multiple times)
  5. Observe order confirmation screen

Depending on your infrastructure, this bug may be challenging to reproduce. It requires all of the "Pay" button clicks to be processed concurrently by Magento. You will not see the bug if Magento is processing requests sequentially (as the second request will reference a cart ID that no longer exists).

Expected result

  1. A single order has been placed, observable via MAP
  2. In access logs / chrome dev tools, the first hit to /rest/ro_uk/V1/carts/mine/payment-information (this endpoint creates the order) receives a HTTP 200 response. Subsequent hits return HTTP 400.
  3. Only one request is sent onto /rest/ro_uk/V1/carts/mine/payment-information (the endpoint that creates an order), which is responded to with HTTP 200.
  4. Alternatively, if multiple requests are unavoidable: Only the first request to /rest/ro_uk/V1/carts/mine/payment-information returns HTTP 200, the rest return HTTP 400

Actual result

  1. Multiple orders have been placed (up to the amount of times the button has been clicked)
  2. In access logs / chrome dev tools, there are multiple hits to /rest/ro_uk/V1/carts/mine/payment-information, which all receive a HTTP 200 response code.

I'm currently working on setting up a dev environment where I can test this, rather than merely observing it on our production instances. I'll come back and add more details once I've got that working.

@orlangur

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2017

Is it not enough to just disable "Pay" button after it is clicked for the first time? Is it possible, for example, to fulfill multiple orders by performing only one payment?

@RJacksonm1

This comment has been minimized.

Copy link
Author

commented Sep 4, 2017

@orlangur That's the fix I'm hoping to implement, but I'm imagining it'll require overriding placeOrderClick in /app/code/Magento/Braintree/view/frontend/web/js/view/payment/method-renderer/hosted-fields.js. I'm not sure if there's a less intrusive way I can fix this for my project? I'd rather avoid overwriting core Payment related code where possible.

I don't think its possible to fulfil multiple orders with a single payment. The sequence of operations is:

  1. Pay button is clicked
  2. AJAX request to BrainTree, returning a payment auth code
  3. AJAX request to /rest/ro_uk/V1/carts/mine/payment-information, which creates the order
  4. Customer is redirected to order success page

Step 3 is hit multiple times if the pay button is clicked multiple times, with different auth codes coming back from Braintree. If multiple orders were fulfilled, I'd expect this to result in multiple charges to the customer (which is still pretty undesirable)

@joni-jones

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2017

Hi, @RJacksonm1, thanks for reporting. I've created an internal ticket (MAGETWO-72351).

maximbaibakov added a commit to maximbaibakov/magento2 that referenced this issue Sep 8, 2017
magento#10767 Fix Braintree payment for multiple charges for the same…
… order, in case, if client click on order button multiple times
@maximbaibakov

This comment has been minimized.

Copy link

commented Sep 8, 2017

Hi @RJacksonm1 and @joni-jones

@RJacksonm1 I feel your pain, as we had some of such issues to our clients.

Please find my PR and commit. I hope the description will give you a decent explanation on why it is happening and how it got addressed.

We have been used the fix for more then a year ago across multiple clients starting from Magento 2.0 till the latest 2.1.8 and had no issues so far.

I think, that should definitely help to over come the issue you are facing.

Regards,
Maxim

@magento-engcom-team

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2017

@RJacksonm1, thank you for your report.
We were not able to reproduce this issue by following the steps you provided. If you'd like to update it, please reopen the issue.
We tested the issue on 2.1.9

@joni-jones

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2017

Closed by mistake. Internal ticket for 2.1.x - MAGETWO-72534.

magento-team pushed a commit that referenced this issue Sep 27, 2017
MAGETWO-72351: [Github] Race condition causing duplicate orders with …
…double-clicks on Braintree "Place Order" button #10767

 - Added button disabling to prevent place order multiple requests
@magento-engcom-team

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2017

@RJacksonm1, thank you for your report.
We've created internal ticket(s) MAGETWO-72351 to track progress on the issue.

@magento-engcom-team

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2017

The issue MAGETWO-72351 already fixed and delivered to 2.2.1 release which will be available soon. Closing the issue

tr33m4n added a commit to tr33m4n/magento2 that referenced this issue Oct 31, 2017
okorshenko pushed a commit that referenced this issue Nov 13, 2017
MAGETWO-83742: [Backport 2.1-develop] #10767 Race condition causing d…
…uplicate orders with double-clicks on Braintree 'Pay' button #11901

 - Merge Pull Request #11901 from tr33m4n/magento2:backport-issue-10767
 - Merged commits:
   1. ddc937f
okorshenko added a commit that referenced this issue Nov 13, 2017
MAGETWO-83742: [Backport 2.1-develop] #10767 Race condition causing d…
…uplicate orders with double-clicks on Braintree "Pay" button #11901
magento-engcom-team pushed a commit that referenced this issue Mar 15, 2018
MAGETWO-72626: [2.3.x][Github] Race condition causing duplicate order…
…s with double-clicks on Braintree "Place Order" button #10767
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.