[FIX] payment_stripe: add idempotency key to prevent multiple payments
Video 1 (Issue): https://drive.google.com/file/d/1oXYcDJgaT9gmhkjE08yJlXwIL1qPwS1Y/view?usp=sharing Issue: When using the register payment with a token with any of the payment acquirers, if there is a concurrent access error during the reconciliation process, the payment intent is sent multiple times to the acquirer, making the card charged multiple times. Steps to reproduce: -Have a V14 database (only tested this version) with sale_mmanagement, payment_stripe and invoicing -Configure Stripe with your public and secret key (2FA is now enforced for Stripe accounts, therefore, we don't have a generic test account anymore. You have to create your own.It is quite fast and easy to do) -Have a portal user PU with an already registered payment token PT -Go to Invoicing -Create a new invoice I: -Customer PU -Add anything in invoice lines -Confirm I -Register a payment for I: -Journal: Stripe -Saved Payment token: PT AT THIS STEP, YOU MUST ENSURE A CONCURRENT ACCESS ERROR WILL RAISE DURING THE RECONCILIATION -Create Payment Log analysis: A first payment intent is sent to Stripe. The card is charged and Stripe answers that all went as expected. We try to process the payment, but a concurrent access error occurs. A retry is done. A payment intent is sent again to Stripe, The card is charged AGAIN and Stripe answers that all went as expected. We try to process the payment, but a concurrent access error occurs. For each retry, the intent is sent and the card is charged. If the first retry succeeds, then Odoo can finish the process. There will be only 1 payment transaction on Odoo's side (others have been rollbacked) but there will be 3 on Stripe's side and the card will be charged 3 times. This PR mitigate this behaviour. It doesn't address the root cause but by adding the idempotency key to the headers with the hash of the transaction reference and the database UUID, we prevent mutliple payments to happen. OPW-2662964 closes odoo/odoo#103515 X-original-commit: 5fe1c1bbba4d754eed7e8927b82752969d6f563d Signed-off-by: Antoine Vandevenne (anv) <anv@odoo.com>
This commit is contained in:
committed by
Antoine Vandevenne (anv)
parent
7501bf3b8e
commit
e9234fd52c
@@ -1,5 +1,7 @@
|
||||
# Part of Odoo. See LICENSE file for full copyright and licensing details.
|
||||
|
||||
from hashlib import sha1
|
||||
|
||||
from odoo import fields
|
||||
from odoo.http import request
|
||||
from odoo.tools import consteq, float_round, ustr
|
||||
@@ -157,3 +159,29 @@ def check_rights_on_recordset(recordset):
|
||||
"""
|
||||
recordset.check_access_rights('write')
|
||||
recordset.check_access_rule('write')
|
||||
|
||||
|
||||
# Idempotency
|
||||
|
||||
def generate_idempotency_key(tx, scope=None):
|
||||
""" Generate an idempotency key for the provided transaction and scope.
|
||||
|
||||
Idempotency keys are used to prevent API requests from going through twice in a short time: the
|
||||
API rejects requests made after another one with the same payload and idempotency key if it
|
||||
succeeded.
|
||||
|
||||
The idempotency key is generated based on the transaction reference, database UUID, and scope if
|
||||
any. This guarantees the key is identical for two API requests with the same transaction
|
||||
reference, database, and endpoint. Should one of these parameters differ, the key is unique from
|
||||
one request to another (e.g., after dropping the database, for different endpoints, etc.).
|
||||
|
||||
:param recordset tx: The transaction to generate an idempotency key for, as a
|
||||
`payment.transaction` record.
|
||||
:param str scope: The scope of the API request to generate an idempotency key for. This should
|
||||
typically be the API endpoint. It is not necessary to provide the scope if the
|
||||
API takes care of comparing idempotency keys per endpoint.
|
||||
:return: The generated idempotency key.
|
||||
:rtype: str
|
||||
"""
|
||||
database_uuid = tx.env['ir.config_parameter'].sudo().get_param('database.uuid')
|
||||
return sha1(f'{database_uuid}{tx.reference}{scope or ""}'.encode()).hexdigest()
|
||||
|
||||
@@ -207,7 +207,9 @@ class PaymentProvider(models.Model):
|
||||
|
||||
# === BUSINESS METHODS - PAYMENT FLOW === #
|
||||
|
||||
def _stripe_make_request(self, endpoint, payload=None, method='POST', offline=False):
|
||||
def _stripe_make_request(
|
||||
self, endpoint, payload=None, method='POST', offline=False, idempotency_key=None
|
||||
):
|
||||
""" Make a request to Stripe API at the specified endpoint.
|
||||
|
||||
Note: self.ensure_one()
|
||||
@@ -216,6 +218,7 @@ class PaymentProvider(models.Model):
|
||||
:param dict payload: The payload of the request
|
||||
:param str method: The HTTP method of the request
|
||||
:param bool offline: Whether the operation of the transaction being processed is 'offline'
|
||||
:param str idempotency_key: The idempotency key to pass in the request.
|
||||
:return The JSON-formatted content of the response
|
||||
:rtype: dict
|
||||
:raise: ValidationError if an HTTP error occurs
|
||||
@@ -228,6 +231,8 @@ class PaymentProvider(models.Model):
|
||||
'Stripe-Version': API_VERSION, # SetupIntent requires a specific version.
|
||||
**self._get_stripe_extra_request_headers(),
|
||||
}
|
||||
if method == 'POST' and idempotency_key:
|
||||
headers['Idempotency-Key'] = idempotency_key
|
||||
try:
|
||||
response = requests.request(method, url, data=payload, headers=headers, timeout=60)
|
||||
# Stripe can send 4XX errors for payment failures (not only for badly-formed requests).
|
||||
|
||||
@@ -218,6 +218,9 @@ class PaymentTransaction(models.Model):
|
||||
'payment_intents',
|
||||
payload=self._stripe_prepare_payment_intent_payload(payment_by_token=True),
|
||||
offline=self.operation == 'offline',
|
||||
idempotency_key=payment_utils.generate_idempotency_key(
|
||||
self, scope='payment_intents_token'
|
||||
), # Prevent multiple offline payments by token (e.g., due to a cursor rollback).
|
||||
)
|
||||
else: # 'online_direct' (express checkout).
|
||||
response = self.provider_id._stripe_make_request(
|
||||
|
||||
Reference in New Issue
Block a user