[IMP] payment(_authorize): rework validation transactions refund flow

Before this commit, the validation flow with verification (payment of a
small amount with immediate refund) was performed with the use of
validation routes: after payment, the customer was redirected to the
validation route stored on the transaction to trigger the refund. This
implementation had an issue: if the customer never reached the
validation route, they were not refunded their validation amount. This
could happen if the customer closed the tab after paying with an
acquirer offering payments with redirection, or if the validation
payment was asynchronously confirmed through a webhook notification.

This commit gets rid of validation routes and requires acquirers to
immediately refund the validation amount when the payment is confirmed.
This way, a payment confirmation coming from a webhook can trigger the
refund too.

As the only acquirer that implements the validation with verification
flow, Authorize.net now voids validation transactions as soon as they
are authorized.

While we're at it, the logging of processing values is adapted to only
log specific rendering values if a redirect form is rendered.

task-2612977

closes odoo/odoo#74707

Related: odoo/enterprise#20060
Related: odoo/upgrade#2710
Signed-off-by: Antoine Vandevenne (anv) <AntoineVDV@users.noreply.github.com>

addons/payment/controllers/portal.py

@@ -32,9 +32,6 @@ class PaymentPortal(portal.CustomerPortal):
       transaction.
     - `/payment/confirmation` is the `landing_route` for the standard payment flow. It displays the
       payment confirmation page to the user when the transaction is validated.
-    - `/payment/validation` is the `validation_route` for the standard payment method validation
-      flow. It redirects the user to `/my/payment_method` in order to display the result and allow
-      to start the flow over.
     """
 
     @http.route(
@@ -170,7 +167,6 @@ class PaymentPortal(portal.CustomerPortal):
             'partner_id': partner.id,
             'access_token': access_token,
             'transaction_route': '/payment/transaction',
-            'validation_route': '/payment/validation',
             'landing_route': '/my/payment_method',
             **self._get_custom_rendering_context_values(**kwargs),
         }
@@ -210,14 +206,12 @@ class PaymentPortal(portal.CustomerPortal):
             amount=amount, currency_id=currency_id, partner_id=partner_id, **kwargs
         )
 
-        # The generic validation and landing routes require the tx id and access token to be
-        # provided, since there is no document to rely on. The access token is recomputed in case
-        # we are dealing with a validation transaction (acquirer-specific amount and currency).
+        # The generic landing route require the tx id and access token to be provided, since there
+        # is no document to rely on. The access token is recomputed in case we are dealing with a
+        # validation transaction (acquirer-specific amount and currency).
         access_token = payment_utils.generate_access_token(
             tx_sudo.partner_id.id, tx_sudo.amount, tx_sudo.currency_id.id
         )
-        tx_sudo.validation_route = tx_sudo.validation_route \
-                                   and f'{tx_sudo.validation_route}&access_token={access_token}'
         tx_sudo.landing_route = f'{tx_sudo.landing_route}' \
                                 f'?tx_id={tx_sudo.id}&access_token={access_token}'
 
@@ -225,7 +219,7 @@ class PaymentPortal(portal.CustomerPortal):
 
     def _create_transaction(
         self, payment_option_id, reference_prefix, amount, currency_id, partner_id, flow,
-        tokenization_requested, validation_route, landing_route, invoice_id=None,
+        tokenization_requested, landing_route, is_validation=False, invoice_id=None,
         custom_create_values=None, **kwargs
     ):
         """ Create a draft transaction based on the payment context and return it.
@@ -240,10 +234,9 @@ class PaymentPortal(portal.CustomerPortal):
         :param int partner_id: The partner making the payment, as a `res.partner` id
         :param str flow: The online payment flow of the transaction: 'redirect', 'direct' or 'token'
         :param bool tokenization_requested: Whether the user requested that a token is created
-        :param str validation_route: The route the user is redirected to in order to refund a
-                                     validation transaction
         :param str landing_route: The route the user is redirected to after the transaction
-        :param int|None invoice_id: The account move for which a payment id made, as a `account.move` id
+        :param bool is_validation: Whether the operation is a validation
+        :param int invoice_id: The account move for which a payment id made, as an `account.move` id
         :param dict custom_create_values: Additional create values overwriting the default ones
         :param dict kwargs: Locally unused data passed to `_is_tokenization_required` and
                             `_compute_reference`
@@ -287,7 +280,7 @@ class PaymentPortal(portal.CustomerPortal):
             **(custom_create_values or {}),
             **kwargs
         )
-        if validation_route:  # Acquirers determine the amount and currency in validation operations
+        if is_validation:  # Acquirers determine the amount and currency in validation operations
             amount = acquirer_sudo._get_validation_amount()
             currency_id = acquirer_sudo._get_validation_currency().id
 
@@ -299,14 +292,11 @@ class PaymentPortal(portal.CustomerPortal):
             'currency_id': currency_id,
             'partner_id': partner_id,
             'token_id': token_id,
-            'operation': f'online_{flow}' if not validation_route else 'validation',
+            'operation': f'online_{flow}' if not is_validation else 'validation',
             'tokenize': tokenize,
-            'validation_route': validation_route,
             'landing_route': landing_route,
             **(custom_create_values or {}),
         })  # In sudo mode to allow writing on callback fields
-        # Validation routes require the transaction id
-        tx_sudo.validation_route = validation_route and f'{validation_route}?tx_id={tx_sudo.id}'
 
         if flow == 'token':
             tx_sudo._send_payment_request()  # Payments by token process transactions immediately
@@ -368,44 +358,6 @@ class PaymentPortal(portal.CustomerPortal):
             # Display the portal homepage to the user
             return request.redirect('/my/home')
 
-    @http.route('/payment/validation', type='http', methods=['GET'], auth='user', website=True)
-    def payment_validation_transaction(self, tx_id, access_token):
-        """ Refund a validation transaction and redirect the user to the landing route.
-
-        :param str tx_id: The validation transaction, as a `payment.transaction` id
-        :param str access_token: The access token used to verify the user
-        """
-        # Raise an HTTP 404 if the tx id or the access token is invalid
-        tx_id = self.cast_as_int(tx_id)
-        tx_sudo = request.env['payment.transaction'].sudo().browse(tx_id).exists()
-        if not tx_sudo or not payment_utils.check_access_token(
-            access_token, tx_sudo.partner_id.id, tx_sudo.amount, tx_sudo.currency_id.id
-        ):
-            raise werkzeug.exceptions.NotFound  # Don't leak info about existence of an id
-
-        self._refund_validation_transaction(tx_id)
-        return request.redirect(tx_sudo.landing_route)
-
-    def _refund_validation_transaction(self, tx_id):
-        """ Refund a validation transaction and remove it from post-processing.
-
-        :param str tx_id: The validation transaction to refund, as a `payment.transaction` id
-        :return: The refunded transaction
-        :rtype: recordset of `payment.transaction`
-        :raise: ValidationError if the transaction id is invalid
-        """
-        tx_id = self.cast_as_int(tx_id)
-        tx_sudo = request.env['payment.transaction'].sudo().browse(tx_id).exists()
-        if not tx_sudo:
-            raise werkzeug.exceptions.NotFound
-
-        if tx_sudo.operation == 'validation':  # Don't allow to refund non-validation transactions
-            tx_sudo._send_refund_request()
-
-        PaymentPostProcessing.remove_transactions(tx_sudo)
-
-        return tx_sudo
-
     @staticmethod
     def cast_as_int(str_value):
         """ Cast a string as an `int` and return it.

addons/payment/models/payment_transaction.py

@@ -83,9 +83,6 @@ class PaymentTransaction(models.Model):
     tokenize = fields.Boolean(
         string="Create Token",
         help="Whether a payment token should be created when post-processing the transaction")
-    validation_route = fields.Char(
-        string="Validation Route",
-        help="The route the user is redirected to in order to refund a validation transaction")
     landing_route = fields.Char(
         string="Landing Route",
         help="The route the user is redirected to after the transaction")
@@ -425,15 +422,15 @@ class PaymentTransaction(models.Model):
 
         # Render the html form for the redirect flow if available
         if self.operation in ('online_redirect', 'validation'):
-            rendering_values = self._get_specific_rendering_values(processing_values)
-            _logger.info(
-                "acquirer-specific rendering values for transaction with id %s:\n%s",
-                self.id, pprint.pformat(rendering_values)
-            )
             redirect_form_view = self.acquirer_id._get_redirect_form_view(
                 is_validation=self.operation == 'validation'
             )
             if redirect_form_view:  # Some acquirer don't need a redirect form
+                rendering_values = self._get_specific_rendering_values(processing_values)
+                _logger.info(
+                    "acquirer-specific rendering values for transaction with id %s:\n%s",
+                    self.id, pprint.pformat(rendering_values)
+                )
                 redirect_form_html = redirect_form_view._render(rendering_values, engine='ir.qweb')
                 processing_values.update(redirect_form_html=redirect_form_html)
 
@@ -677,8 +674,8 @@ class PaymentTransaction(models.Model):
     def _send_refund_request(self):
         """ Request the provider of the acquirer handling the transaction to refund it.
 
-        For an acquirer to support tokenization, it must override this method and request a refund
-        to its provider *if the validation amount is not null*.
+        For an acquirer to support refunds, it must override this method and request a refund
+        to its provider.
 
         Note: self.ensure_one()
 
@@ -744,9 +741,7 @@ class PaymentTransaction(models.Model):
             'currency_code': self.currency_id.name,
             'state': self.state,
             'state_message': self.state_message,
-            'is_validation': self.operation == 'validation',
             'is_post_processed': self.is_post_processed,
-            'validation_route': self.validation_route,
             'landing_route': self.landing_route,
         }
         _logger.debug(

addons/payment/static/src/js/manage_form.js

@@ -208,6 +208,7 @@ odoo.define('payment.manage_form', require => {
             this._disableButton(true); // Disable until it is needed again
             if (flow !== 'token') { // Creation of a new token
                 this.txContext.tokenizationRequested = true;
+                this.txContext.isValidation = true;
                 this._processPayment(provider, paymentOptionId, flow);
             } else if (this.txContext.allowTokenSelection) { // Assignation of a token to a record
                 this._assignToken(paymentOptionId);

addons/payment/static/src/js/payment_form_mixin.js

@@ -301,9 +301,8 @@ odoo.define('payment.payment_form_mixin', require => {
                     ? parseInt(this.txContext.invoiceId) : null,
                 'flow': flow,
                 'tokenization_requested': this.txContext.tokenizationRequested,
-                'validation_route': this.txContext.validationRoute
-                    ? this.txContext.validationRoute : null,
                 'landing_route': this.txContext.landingRoute,
+                'is_validation': this.txContext.isValidation,
                 'access_token': this.txContext.accessToken
                     ? this.txContext.accessToken : undefined,
                 'csrf_token': core.csrf_token,

addons/payment/static/src/js/post_processing.js

@@ -80,11 +80,7 @@ odoo.define('payment.post_processing', function (require) {
                 // In almost every cases there will be a single transaction to display. If there are
                 // more than one transaction, the last one will most likely be the one that was
                 // confirmed. We use this one to redirect the user to the final page.
-                if (display_values_list[0].is_validation) {
-                    window.location = display_values_list[0].validation_route;
-                } else {
-                    window.location = display_values_list[0].landing_route;
-                }
+                window.location = display_values_list[0].landing_route;
                 return;
             }
 

addons/payment/tests/http_common.py

@@ -178,19 +178,3 @@ class PaymentHttpCommon(PaymentTestUtils, HttpCase):
 
         resp_content = json.loads(response.content)
         return resp_content['result']
-
-    # payment/validation #
-    ######################
-    def portal_validate_transaction(self, tx):
-        """/payment/validation feedback, for a given tx (and its validation_route)
-
-        NOTE: the validation route is restricted to logged users
-
-        If you are not logged in, the route won't be called
-        but it won't raise/log, the response will be a redirection to the login page.
-        """
-        uri = tx.validation_route
-        url = self._build_url(uri)
-        # No params since all GET arguments are already specified in the validation route.
-        # no returned value, validation response = redirect
-        return self._make_http_get_request(url, {})

addons/payment/tests/test_flows.py

@@ -1,7 +1,5 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from unittest.mock import patch
-
 from freezegun import freeze_time
 
 from odoo.tests import tagged
@@ -46,7 +44,6 @@ class TestFlows(PaymentCommon, PaymentHttpCommon):
             'flow': flow,
             'payment_option_id': self.acquirer.id,
             'tokenization_requested': False,
-            'validation_route': False,
         })
 
         if flow == 'token':
@@ -140,8 +137,7 @@ class TestFlows(PaymentCommon, PaymentHttpCommon):
     # VALIDATION #
     ##############
 
-    # NOTE: not tested as public user because the validation route & flow
-    #   is only available for logged in users.
+    # NOTE: not tested as public user because the validation flow is only available when logged in
 
     # freeze time for consistent singularize_prefix behavior during the test
     @freeze_time("2011-11-02 12:00:21")
@@ -169,10 +165,10 @@ class TestFlows(PaymentCommon, PaymentHttpCommon):
             'access_token': tx_context['access_token'],
             'flow': flow,
             'payment_option_id': self.acquirer.id,
-            'tokenization_requested': False,
-            'validation_route': tx_context['validation_route'],
+            'tokenization_requested': True,
             'reference_prefix': tx_context['reference_prefix'],
             'landing_route': tx_context['landing_route'],
+            'is_validation': True,
         }
         with mute_logger('odoo.addons.payment.models.payment_transaction'):
             processing_values = self.get_processing_values(**transaction_values)
@@ -191,23 +187,6 @@ class TestFlows(PaymentCommon, PaymentHttpCommon):
         self.assertEqual(processing_values['partner_id'], self.partner.id)
         self.assertEqual(processing_values['reference'], expected_reference)
 
-        _send_refund_request = type(self.env['payment.transaction'])._send_refund_request
-
-        def do_sthg_on_refund(self):
-            self.state_message = "refund"
-            return _send_refund_request(self)
-        with patch.object(
-            type(self.env['payment.transaction']),
-            '_send_refund_request',
-            do_sthg_on_refund,
-        ):
-            self.portal_validate_transaction(tx_sudo)
-
-        # Request was in another thread (for the request)
-        # We need to invalidate current cache to fetch latest value from db.
-        tx_sudo.invalidate_cache(['state_message'])
-        self.assertEqual(tx_sudo.state_message, 'refund')
-
     def test_51_validation_direct_portal(self):
         self.authenticate(self.portal_user.login, self.portal_user.login)
         self.partner = self.portal_partner
@@ -327,7 +306,6 @@ class TestFlows(PaymentCommon, PaymentHttpCommon):
             'flow': 'direct',
             'payment_option_id': self.acquirer.id,
             'tokenization_requested': False,
-            'validation_route': False,
         })
         with mute_logger('odoo.addons.payment.models.payment_transaction'):
             processing_values = self.get_processing_values(**route_values)
@@ -344,7 +322,6 @@ class TestFlows(PaymentCommon, PaymentHttpCommon):
             'flow': 'this flow does not exist',
             'payment_option_id': self.acquirer.id,
             'tokenization_requested': False,
-            'validation_route': 'whatever',
             'reference_prefix': 'whatever',
             'landing_route': 'whatever',
         })

addons/payment/tests/test_multicompany_flows.py

@@ -51,7 +51,6 @@ class TestMultiCompanyFlows(PaymentMultiCompanyCommon, PaymentHttpCommon):
             'flow': 'direct',
             'payment_option_id': self.acquirer_company_b.id,
             'tokenization_requested': False,
-            'validation_route': False,
         })
         with mute_logger('odoo.addons.payment.models.payment_transaction'):
             processing_values = self.get_processing_values(**validation_values)

addons/payment/views/payment_templates.xml

@@ -151,8 +151,6 @@
                                      enables the token assignation mechanisms: creation of a new
                                      token through a refunded transaction and assignation of an
                                      existing token
-            - 'validation_route' - The route the user is redirected to in order to refund the
-                                   validation transaction
             - 'landing_route' - The route the user is redirected to at then end of the flow
             - 'footer_template_id' - The template id for the submit button. Optional
         -->
@@ -164,7 +162,6 @@
               t-att-data-invoice-id="invoice_id"
               t-att-data-transaction-route="transaction_route"
               t-att-data-assign-token-route="assign_token_route"
-              t-att-data-validation-route="validation_route"
               t-att-data-landing-route="landing_route"
               t-att-data-allow-token-selection="bool(assign_token_route)">
             <t t-set="acquirer_count" t-value="len(acquirers) if acquirers else 0"/>

addons/payment_authorize/models/payment_transaction.py

@@ -131,6 +131,7 @@ class PaymentTransaction(models.Model):
                 self._set_authorized()
                 if self.tokenize and not self.token_id:
                     self._authorize_tokenize()
+                self._send_refund_request()  # In last step because it calls _handle_feedback_data()
             elif status_type == 'void':
                 if self.operation == 'validation':  # Validation txs are authorized and then voided
                     self._set_done()  # If the refund went through, the validation tx is confirmed