Back to home

Two tabs, two charges: a race in Webpay and how to close it

A payment flow can pass all its tests and still charge twice. It happened to me with a Webpay Plus integration in Laravel for paid bookings (Webpay Plus is Transbank's card payment gateway in Chile). The normal path was tested end to end against Transbank's integration environment and it worked. The failure showed up when I reviewed the code looking for what happens if the same user starts the payment twice.

It was not an incident. I found it in an audit of my own code, before it happened to a customer, and I reproduced it with a test. I'm writing it up because the cause is easy to repeat in any integration.

The sequence

  1. The user opens the booking in two tabs, or double-clicks “Pay”.
  2. Each request asks whether the booking already has an approved payment. Neither does yet, so both create an “in progress” transaction.
  3. The user completes the Webpay form in both tabs.
  4. Each return confirms its own transaction. Both end up approved and the card is charged twice.

Two tabs pay for the same booking with no lock. Tab A and tab B follow the same steps: they start the payment, pay on Webpay and return to the site. Both confirmations run at the same time. Both end up approved and the card is charged twice.

Before: with no lock on the booking, both transactions are confirmed.

The two causes

The first is a race when creating the attempt. The code read the booking's state and then wrote the transaction, with nothing locked in between. Two nearly simultaneous requests see the same state and both go ahead.

The second is less obvious. On return, each transaction was validated only against itself: that the order, the amount and the session matched what was stored. All of that was correct, and still nobody asked whether the booking already had another approved transaction. The business rule (“one booking, one approved payment”) was not written anywhere in the code.

The fix

There are two independent defenses, one at each end of the flow.

When creating the attempt, the booking row is locked inside the database transaction. The two requests go through one after the other, and the second already sees the first one's attempt.

On return, the transaction is locked first and then the booking. If another transaction of the same booking is already approved, the current one is cancelled and not confirmed with Transbank:

start database transaction
  lock(transaction)
  if already approved: reply "already confirmed" and exit
  lock(booking)
  if the booking has another approved transaction:
    cancel this transaction, without confirming with Transbank
    reply "already paid by another attempt"
  confirm with Transbank
  compare order, amount and session with what is stored
  mark approved
end database transaction

The lock on the booking forces two simultaneous confirmations to run one after the other. The second finds the first one's approval, is cancelled and is not confirmed with Transbank.

Two tabs pay for the same booking. Tab A and tab B follow the same steps: they start the payment, pay on Webpay and return to the site. Both confirmations go through a lock on the booking, one after the other. Tab A is confirmed. Tab B is cancelled: already paid by another attempt.

After: with a lock on the booking, only one attempt is confirmed.

A detail that took me a second pass: the lock has to be inside the same database transaction as the read and the write. A lock taken outside is released too early and protects nothing.

There is a third defense that I use in another project and that avoids the problem earlier: if the order already has a pending attempt with the same provider, it is reused instead of creating a new one. That way the user never ends up with two payment forms open for the same thing.

How to test it without threads

A race looks hard to test, but this one doesn't need real concurrency. The test seeds two in-progress transactions for the same booking, simulates Transbank approving both, and runs both through the return. The expected result is one approved and one cancelled. It is deterministic and runs in milliseconds.

The end-to-end test goes as far as the redirect to Webpay and checks the amount. It does not complete the card form: that is a third party's interface, and the integration environment sometimes fails on its own. Continuous integration should not depend on it.

Other things the Webpay return does that you should expect

The return is an input you don't control, and it can arrive more than once. These are the cases I had to handle:

  • It arrives twice. If the transaction is already approved, the response is the same as before, with no second confirmation and no resent emails.
  • It arrives without a token. The form has three endings: the user pays, the user cancels, or the form expires without notice. The third carries neither the payment token nor the cancellation token. In that case I query the transaction's status at Transbank and close the attempt, or confirm it if it managed to get authorized.
  • The confirmation fails halfway. If the confirmation call throws an error, I don't assume the payment failed. I query the status and decide from that.
  • The attempt is abandoned. A transaction nobody finished can keep showing as started for a long time. If that blocks the customer from paying again, it has to be closed on a timeout, after checking that order, session and amount match.
  • Emails go outside the transaction. An email can't be undone. If it is sent inside the database transaction and something fails afterwards, the database rolls back and the email is already out.

Summary

  • Validating the transaction against itself is not enough. The business rule applies over whatever groups the payments: the booking, the order, the purchase.
  • Lock when creating the attempt and also when confirming it, always inside the database transaction.
  • Treat the return as a repeatable, untrusted input.
  • Test the race by seeding the state, not with threads.

If you have a payment flow to review, write to me at contacto@videla.dev.