From a6a8a669b973c2a54227d3a8337de92750afc33f Mon Sep 17 00:00:00 2001 From: Demesmaeker Date: Wed, 16 Feb 2022 09:13:44 +0000 Subject: [PATCH] [IMP] payment_stripe: manage async notifications Some payments may be confirmed asynchronously, and we need to be aware of their status, even if they are not in a "checkout session complete" event. Relying on successful payment or setup intents instead manage more use cases. task-2733725 closes odoo/odoo#84150 Related: odoo/upgrade#3350 Signed-off-by: Antoine Vandevenne (anv) --- addons/payment_stripe/const.py | 6 +- addons/payment_stripe/controllers/main.py | 37 +++++------- .../payment_stripe/models/payment_acquirer.py | 4 +- .../models/payment_transaction.py | 5 +- addons/payment_stripe/tests/common.py | 59 +++---------------- addons/payment_stripe/tests/test_stripe.py | 34 ++++++++--- 6 files changed, 57 insertions(+), 88 deletions(-) diff --git a/addons/payment_stripe/const.py b/addons/payment_stripe/const.py index 877e2e2b261..f7cfd2f241e 100644 --- a/addons/payment_stripe/const.py +++ b/addons/payment_stripe/const.py @@ -29,6 +29,8 @@ INTENT_STATUS_MAPPING = { } # Events which are handled by the webhook -WEBHOOK_HANDLED_EVENTS = [ - 'checkout.session.completed', +HANDLED_WEBHOOK_EVENTS = [ + 'payment_intent.amount_capturable_updated', + 'payment_intent.succeeded', + 'setup_intent.succeeded', ] diff --git a/addons/payment_stripe/controllers/main.py b/addons/payment_stripe/controllers/main.py index d57116ded4c..0253b973fae 100644 --- a/addons/payment_stripe/controllers/main.py +++ b/addons/payment_stripe/controllers/main.py @@ -13,6 +13,8 @@ from odoo import http from odoo.exceptions import ValidationError from odoo.http import request +from odoo.addons.payment_stripe.const import HANDLED_WEBHOOK_EVENTS + _logger = logging.getLogger(__name__) @@ -84,36 +86,27 @@ class StripeController(http.Controller): event = json.loads(request.httprequest.data) _logger.info("notification received from Stripe with data:\n%s", pprint.pformat(event)) try: - if event['type'] == 'checkout.session.completed': - checkout_session = event['data']['object'] + if event['type'] in HANDLED_WEBHOOK_EVENTS: + intent = event['data']['object'] # PaymentIntent/SetupIntent, depending on the flow # Check the integrity of the event - data = {'reference': checkout_session['client_reference_id']} + data = {'reference': intent['description']} tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_notification_data( 'stripe', data ) self._verify_notification_signature(tx_sudo) - # Fetch the PaymentIntent, Charge and PaymentMethod objects from Stripe - if checkout_session.get('payment_intent'): # Can be None - payment_intent = tx_sudo.acquirer_id._stripe_make_request( - f'payment_intents/{tx_sudo.stripe_payment_intent}', method='GET' + if event['type'].startswith('payment_intent'): + self._include_payment_intent_in_notification_data(intent, data) + else: # Validation + # Fetch the missing PaymentMethod object. + payment_method = tx_sudo.acquirer_id._stripe_make_request( + f'payment_methods/{intent["payment_method"]}', method='GET' ) _logger.info( - "received payment_intents response:\n%s", pprint.pformat(payment_intent) + "received payment_methods response:\n%s", pprint.pformat(payment_method) ) - self._include_payment_intent_in_notification_data(payment_intent, data) - - # Fetch the SetupIntent and PaymentMethod objects from Stripe - if checkout_session.get('setup_intent'): # Can be None - setup_intent = tx_sudo.acquirer_id._stripe_make_request( - f'setup_intents/{checkout_session.get("setup_intent")}', - payload={'expand[]': 'payment_method'}, - method='GET' - ) - _logger.info( - "received setup_intents response:\n%s", pprint.pformat(setup_intent) - ) - self._include_setup_intent_in_notification_data(setup_intent, data) + intent['payment_method'] = payment_method + self._include_setup_intent_in_notification_data(intent, data) # Handle the notification data crafted with Stripe API objects tx_sudo._handle_notification_data('stripe', data) @@ -135,7 +128,7 @@ class StripeController(http.Controller): def _include_setup_intent_in_notification_data(setup_intent, notification_data): notification_data.update({ 'setup_intent': setup_intent, - 'payment_method': setup_intent.get('payment_method') + 'payment_method': setup_intent.get('payment_method'), }) def _verify_notification_signature(self, tx_sudo): diff --git a/addons/payment_stripe/models/payment_acquirer.py b/addons/payment_stripe/models/payment_acquirer.py index af843c48033..db49fc01ec0 100644 --- a/addons/payment_stripe/models/payment_acquirer.py +++ b/addons/payment_stripe/models/payment_acquirer.py @@ -9,7 +9,7 @@ from werkzeug.urls import url_join, url_encode from odoo import _, api, fields, models from odoo.exceptions import ValidationError -from odoo.addons.payment_stripe.const import API_VERSION, PROXY_URL, WEBHOOK_HANDLED_EVENTS +from odoo.addons.payment_stripe.const import API_VERSION, PROXY_URL, HANDLED_WEBHOOK_EVENTS from odoo.addons.payment_stripe.controllers.onboarding import OnboardingController from odoo.addons.payment_stripe.controllers.main import StripeController @@ -133,7 +133,7 @@ class PaymentAcquirer(models.Model): webhook = self._stripe_make_request( 'webhook_endpoints', payload={ 'url': self._get_stripe_webhook_url(), - 'enabled_events[]': WEBHOOK_HANDLED_EVENTS, + 'enabled_events[]': HANDLED_WEBHOOK_EVENTS, 'api_version': API_VERSION, } ) diff --git a/addons/payment_stripe/models/payment_transaction.py b/addons/payment_stripe/models/payment_transaction.py index a9907d72876..9f42d91d07f 100644 --- a/addons/payment_stripe/models/payment_transaction.py +++ b/addons/payment_stripe/models/payment_transaction.py @@ -118,6 +118,7 @@ class PaymentTransaction(models.Model): 'mode': 'setup', 'success_url': return_url, 'cancel_url': return_url, + 'setup_intent_data[description]': self.reference, } ) return checkout_session @@ -155,7 +156,6 @@ class PaymentTransaction(models.Model): """ return { **pmt_values, - 'client_reference_id': self.reference, # Assign a customer to the session so that Stripe automatically attaches the payment # method to it in a validation flow. In checkout flow, a customer is automatically # created if not provided but we still do it here to avoid requiring the customer to @@ -384,8 +384,7 @@ class PaymentTransaction(models.Model): payment_method_id = notification_data.get('charge', {}).get('payment_method') customer_id = notification_data.get('charge', {}).get('customer') else: # 'validation' - payment_method_id = notification_data.get('setup_intent', {}) \ - .get('payment_method', {}).get('id') + payment_method_id = notification_data.get('payment_method', {}).get('id') customer_id = notification_data.get('setup_intent', {}).get('customer') payment_method = notification_data.get('payment_method') if not payment_method_id or not payment_method: diff --git a/addons/payment_stripe/tests/common.py b/addons/payment_stripe/tests/common.py index a8d5c876bdf..2d460d613ee 100644 --- a/addons/payment_stripe/tests/common.py +++ b/addons/payment_stripe/tests/common.py @@ -18,58 +18,15 @@ class StripeCommon(PaymentCommon): cls.acquirer = cls.stripe cls.notification_data = { - 'api_version': '2019-05-16', - 'created': 1326853478, 'data': { 'object': { - 'after_expiration': None, - 'allow_promotion_codes': None, - 'amount_subtotal': None, - 'amount_total': None, - 'automatic_tax': { - 'enabled': False, - 'status': None, - }, - 'billing_address_collection': None, - 'cancel_url': 'https://example.com/payment/stripe/checkout_return?reference=Test Transaction', - 'client_reference_id': cls.reference, - 'consent': None, - 'consent_collection': None, - 'currency': None, - 'customer': None, - 'customer_creation': None, - 'customer_details': None, - 'customer_email': None, - 'expires_at': 1642614393, - 'id': 'cs_00000000000000', - 'livemode': False, - 'locale': None, - 'metadata': {}, - 'mode': 'payment', - 'object': 'checkout.session', - 'payment_intent': 'pi_00000000000000', - 'payment_method_options': {}, - 'payment_method_types': ['card'], - 'payment_status': 'unpaid', - 'phone_number_collection': {'enabled': False}, - 'recovered_from': None, - 'setup_intent': None, - 'shipping': None, - 'shipping_address_collection': None, - 'shipping_options': [], - 'shipping_rate': None, - 'status': 'expired', - 'submit_type': None, - 'subscription': None, - 'success_url': 'https://example.com/payment/stripe/checkout_return?reference=Test Transaction', - 'total_details': None, - 'url': None, - }, + 'id': 'pi_3KTk9zAlCFm536g81Wy7RCPH', + 'charges': {'data': [{'amount': 36800}]}, + 'customer': 'cus_LBxMCDggAFOiNR', + 'payment_method': 'pm_1KVZSNAlCFm536g8sYB92I1G', + 'description': cls.reference, + 'status': 'succeeded', + } }, - 'id': 'evt_00000000000000', - 'livemode': False, - 'object': 'event', - 'pending_webhooks': 1, - 'request': None, - 'type': 'checkout.session.completed', + 'type': 'payment_intent.succeeded' } diff --git a/addons/payment_stripe/tests/test_stripe.py b/addons/payment_stripe/tests/test_stripe.py index d760c668ddc..a159a27fa01 100644 --- a/addons/payment_stripe/tests/test_stripe.py +++ b/addons/payment_stripe/tests/test_stripe.py @@ -68,14 +68,36 @@ class StripeTest(StripeCommon, PaymentHttpCommon): with patch( 'odoo.addons.payment_stripe.controllers.main.StripeController' '._verify_notification_signature' - ), patch( - 'odoo.addons.payment_stripe.models.payment_acquirer.PaymentAcquirer' - '._stripe_make_request', - return_value={'status': 'succeeded'}, ): self._make_json_request(url, data=self.notification_data) self.assertEqual(tx.state, 'done') + @mute_logger('odoo.addons.payment_stripe.controllers.main') + def test_webhook_notification_tokenizes_payment_method(self): + """ Test the processing of a webhook notification. """ + self.create_transaction('dummy', operation='validation', tokenize=True) + url = self._build_url(StripeController._webhook_url) + payment_method_response = { + 'card': {'last4': '4242'}, + 'id': 'pm_1KVZSNAlCFm536g8sYB92I1G', + 'type': 'card' + } + with patch( + 'odoo.addons.payment_stripe.controllers.main.StripeController' + '._verify_notification_signature' + ), patch( + 'odoo.addons.payment_stripe.models.payment_acquirer.PaymentAcquirer' + '._stripe_make_request', + return_value=payment_method_response, + ), patch( + 'odoo.addons.payment_stripe.models.payment_transaction.PaymentTransaction' + '._stripe_tokenize_from_notification_data' + ) as tokenize_check_mock: + self._make_json_request( + url, data=dict(self.notification_data, type="setup_intent.succeeded") + ) + self.assertEqual(tokenize_check_mock.call_count, 1) + @mute_logger('odoo.addons.payment_stripe.controllers.main') def test_webhook_notification_triggers_signature_check(self): """ Test that receiving a webhook notification triggers a signature check. """ @@ -85,10 +107,6 @@ class StripeTest(StripeCommon, PaymentHttpCommon): 'odoo.addons.payment_stripe.controllers.main.StripeController' '._verify_notification_signature' ) as signature_check_mock, patch( - 'odoo.addons.payment_stripe.models.payment_acquirer.PaymentAcquirer' - '._stripe_make_request', - return_value={}, - ), patch( 'odoo.addons.payment.models.payment_transaction.PaymentTransaction' '._handle_notification_data' ):