From e9234fd52c8cc24d64195ef17e5f478297991b0d Mon Sep 17 00:00:00 2001 From: mafo-odoo Date: Tue, 27 Sep 2022 09:00:02 +0000 Subject: [PATCH] [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) --- addons/payment/utils.py | 28 +++++++++++++++++++ .../payment_stripe/models/payment_provider.py | 7 ++++- .../models/payment_transaction.py | 3 ++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/addons/payment/utils.py b/addons/payment/utils.py index 32ebf969cd0..f66edcb9db1 100644 --- a/addons/payment/utils.py +++ b/addons/payment/utils.py @@ -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() diff --git a/addons/payment_stripe/models/payment_provider.py b/addons/payment_stripe/models/payment_provider.py index 54b6b4c4de4..663965480be 100644 --- a/addons/payment_stripe/models/payment_provider.py +++ b/addons/payment_stripe/models/payment_provider.py @@ -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). diff --git a/addons/payment_stripe/models/payment_transaction.py b/addons/payment_stripe/models/payment_transaction.py index 218a6a597b3..1a60b12c658 100644 --- a/addons/payment_stripe/models/payment_transaction.py +++ b/addons/payment_stripe/models/payment_transaction.py @@ -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(