Stripe Payment Intents and Duplicate Charges

Bug #2057948 reported by Michele Morgan
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Committed
High
Unassigned
3.11
Fix Released
High
Unassigned
3.12
Fix Released
High
Unassigned

Bug Description

Evergreen 3.10.3

We have seen a few instances of duplicate charges to the patron for the same Evergreen bill.

To prevent duplicates, Stripe suggests including an idempotency key as an element in the payment request:

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

Another suggestion from Stripe support was to implement a prebuilt payment element, though I'm not sure if this is an option:

https://docs.stripe.com/payments/payment-element

Related:

Mailing list thread: http://list.evergreen-ils.org/pipermail/evergreen-general/2024-March/002465.html

bug 1894005 - Wishlist: Add support for Stripe Payment Intents
bug 1965579 - Stripe Payment Intents and Negative Bills
bug 1981628 - Follow up to Stripe payment intents bug

Changed in evergreen:
status: New → Confirmed
importance: Undecided → High
Revision history for this message
Terran McCanna (tmccanna) wrote :

Marking this to high since although it doesn't happen frequently, it is such a terrible impact to the patrons when it does happen.

Revision history for this message
Ken Cox (kenstir) wrote :

I pushed a patch that should add an Idempotency-Key header to the Stripe /payment_intents request.

The Idempotency-Key is md5_hex(sort(transaction_ids)). This should guarantee that repeated requests to pay the same set of fines is not reposted. It will not prevent a user from paying two fines then trying to pay only the 2nd one again.

tags: added: pullrequest
Revision history for this message
Ken Cox (kenstir) wrote :
Revision history for this message
Michele Morgan (mmorgan) wrote :

Thanks for working on this, Ken!

I've tested the patch with testing keys set up in the credit.processor.stripe.pubkey and credit.processor.stripe.secretkey library settings. Unfortunately, I'm getting an error when I attempt to pay a charge while logged into the opac:

We are unable to process credit card payments at this time. We apologize for the inconvenience. Please contact the library for further assistance.

Evergreen logs show the following:

[2024-03-20 08:57:22] /usr/sbin/apache2 [ERR :2562498:Account.pm:2382:1710939128256249864] Error initializing Stripe: $VAR1 = {'message' => 'Received unknown parameter: -idempotency_key','param' => '-idempotency_key','type' => 'invalid_request_error','code' => 'parameter_unknown','doc_url' => 'https://stripe.com/docs/error-codes/parameter-unknown','request_log_url' => 'https://dashboard.stripe.com/test/logs/req_sUuehM0Re3UX60?t=1710939442'};

On the Stripe side, logs show the following (redacted):

Response body
{
  "error": {
    "code": "parameter_unknown",
    "doc_url": "https://stripe.com/docs/error-codes/parameter-unknown",
    "message": "Received unknown parameter: -idempotency_key",
    "param": "-idempotency_key",
    "request_log_url": "https://dashboard.stripe.com/test/logs/req_xxxxx....",
    "type": "invalid_request_error"
  }
}
Request POST body
{
  "currency": "usd",
  "description": "User Database ID: xxxxxxx",
  "-idempotency_key": "d41d8cd98f00b204e9800998ecf8427e",
  "amount": "495"
}

tags: added: needswork
removed: pullrequest
Revision history for this message
Ken Cox (kenstir) wrote :

Sorry about that! I did belatedly question the wisdom of changing something I didn't know how to test. Looking now...

Revision history for this message
Michele Morgan (mmorgan) wrote :

Ken,

Some further testing revealed that your patch DOES indeed work IF the Business/Stripe.pm file installed on the Evergreen server is the same as you reference in your commit:

https://github.com/aquaron/Business-Stripe/blob/master/Stripe.pm

But the version of Business/Stripe.pm installed on our Ubuntu 22.04 test server is different, and makes no reference to payment intents. The version on our test server appears to be:

https://metacpan.org/dist/Business-Stripe/source/Stripe.pm

No apologies necessary! but why would the github version be different than the metacpan version?

Revision history for this message
Ken Cox (kenstir) wrote :

My bad. The Business::Stripe CPAN module is 0.07, which is not the same as https://github.com/aquaron/Business-Stripe, which is also 0.07 and has the new options.

I reproduced the problem locally and will update the patch.

Changed in evergreen:
assignee: nobody → Ken Cox (kenstir)
Revision history for this message
Ken Cox (kenstir) wrote :

Ha! I missed your update while I was deep in the weeds. I think the maintainer did not update CPAN, which I asked for in https://github.com/aquaron/Business-Stripe/issues/25 . I am trying to make it work with the existing Stripe package. If I can, that would be preferable due to the fewest number of moving parts.

Revision history for this message
Ken Cox (kenstir) wrote :

I pushed a new commit to the same place. I was able to reproduce the problem locally on Ubuntu 22 LTS and fix it. I was not able to test through EG because I failed configuring my server for CC payments. Here's a screenshot of what I did, but the patron with the fine did not have any payment options visible in the OPAC.

tags: added: pullrequest
removed: needswork
Revision history for this message
Ken Cox (kenstir) wrote :
Revision history for this message
Michele Morgan (mmorgan) wrote :

Hi Ken,

I have done some testing on your latest patch. I'm able confirm that the idempotency key is being generated and sent to Stripe when the request is sent.

However, repeated attempts to pay different combinations of circulation charges are failing - all have the same idempotency key.

I discovered that, in Account.pm, 'xact' that you referenced in your commit message represents the selected circulation charges and 'xact_misc' represents the miscellaneous (aka grocery) charges.

If I chose only circulation charges, the idempotency key was a hash of an empty string.

So I think if @payment_xacts aggregates all the charge ids - 'xact' in addition to 'xact_misc', that should do the trick.

Thanks for your work on this!

Revision history for this message
Ken Cox (kenstir) wrote :

Michele, thank you so much for the thorough review.

I pushed a new commit to user/kenstir/lp2057948_add_stripe_idempotency_key that does what you suggest. In fact it is what the SmartPAY code does just 40 lines down, and since it was committed by a Jason I should have known it was good and borrowed it straight up. I'll test more tomorrow. I saw something unexpected in my testing, where the grocery transaction id showed up in the URL, and then navigating back in the browser caused a 2nd stripe request (which errored due to the Idempotency-Key).

Revision history for this message
Terran McCanna (tmccanna) wrote :

>>navigating back in the browser caused a 2nd stripe request<<

Oh!!! That could definitely explain some of the problems we've seen!

Revision history for this message
Michele Morgan (mmorgan) wrote :

Hi Ken,

I've done quite a bit of testing on this today, and it's looking very good when users select individual charges to pay. The idempotency key is getting generated as it should based on the selected charges, and Stripe is recognizing that multiple payment intents on the same combination link back to the original request. That's all working great.

When I tested using the "Pay All Charges" option in the opac, however, the idempotency key is generated from an empty string, so if ANY user, after the first, chooses "Pay All Charges", they get the error:

"We are unable to process credit card payments at this time. We apologize for the inconvenience. Please contact the library for further assistance."

Note that my test server uses tpac, I have not tested this in BOOPAC yet.

Revision history for this message
Ken Cox (kenstir) wrote :

Thanks, Michele. I've done some testing as well and here are my thoughts:

1) If there are no transactions selected, the idempotency key should incorporate the patron ID and the amount and the date. That way two users could pay the same amount on the same day with Pay All Charges, and one user could pay different amounts on the same day or the same amount on different days. I plan to do this.

2) There is still something fishy that causes "While you were trying to make payments, this account's transaction history changed. Please go back and try again.". I believe this was the scenario:
- pressed Pay Selected Charges
- in my debug logs I see "creating intent for xact=487,489 key=dacd248e707c09e30014e18a61df2da1"
- on the CC entry screen pressed back
- pressed Pay Selected Charges
- in my debug logs I see "creating intent for xact=487,489 key=dacd248e707c09e30014e18a61df2da1"
- entered the CC and submitted
- Stripe says the payment was successful
- EG says it wasn't: "While you were trying to make payments, this account's transaction history changed." (Interestingly that check was targeted at this same issue. See commit c5d36d2a99e45b9a02d00a1e795b66a38c271b43.)

Now if I retry the same payment, I get "A processing error occurred."

Revision history for this message
Ken Cox (kenstir) wrote :

Pushed a new commit to branch user/kenstir/lp2057948_add_stripe_idempotency_key

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/kenstir/lp2057948_add_stripe_idempotency_key

This commit fixes the issue above, where an empty transaction list led to a stable idempotency key.

Tested and worked this time with no errors, once when I used the browser back key from the CC form and once where I didn't.

Revision history for this message
Michele Morgan (mmorgan) wrote :

Thanks for your work on this, Ken! I've tested the latest patch using Stripe test credentials while logged into both tpac and boopac. I tested clicking Pay All Charges, as well as Pay Selected Charges with circulation, miscellaneous, and combinations of both types of charges selected.

I initially did not enter credit card info, and logged out or pressed the Back arrow, then attempted to select and pay the same charges again. In all cases, Stripe logs showed that the idempotency key linked repeated attempts to pay the same charges back to the original charge request. All payments processed successfully on the Stripe side as well as the Evergreen side.

Here's a link to my signoff branch:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/mmorgan/lp2057948_signoff

user/mmorgan/lp2057948_signoff

tags: added: signedoff
Revision history for this message
Terran McCanna (tmccanna) wrote :

I'm very happy to get this fix! Thank you Ken and Michele!!

Fix committed as far back as rel_3_11

Changed in evergreen:
assignee: Ken Cox (kenstir) → Terran McCanna (tmccanna)
milestone: none → 3.13-beta
milestone: 3.13-beta → 3.12.3
milestone: 3.12.3 → 3.next
no longer affects: evergreen/3.13
Changed in evergreen:
milestone: 3.next → 3.13-beta
status: Confirmed → Fix Committed
assignee: Terran McCanna (tmccanna) → nobody
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Bug attachments

Remote bug watches

Bug watches keep track of this bug in other bug trackers.