From 4d1a1f1c99d6055b60921ce464b72f0f4d9bbfe2 Mon Sep 17 00:00:00 2001 From: "Anita (anko)" Date: Fri, 17 Mar 2023 14:21:07 +0100 Subject: [PATCH] [IMP] payment, *: verify and reroute payment flows This commit changes the way transactions linked to a document (sales order, invoice...) are created in a payment flow. Rather than receiving and trusting the transaction values from the controller, they are now read from the linked document, and the payment flow is rerouted to use the document's module's controllers instead of that of `payment`. This ensures that no unexpected value can be passed to the `create` method of a transaction, and simplifies the implementation of the payment flows of linked documents. task-3136240 closes odoo/odoo#126425 Related: odoo/enterprise#43212 Related: odoo/upgrade#5124 Signed-off-by: Antoine Vandevenne (anv) --- addons/account_payment/__manifest__.py | 6 -- addons/account_payment/controllers/payment.py | 50 +++++----- addons/account_payment/controllers/portal.py | 1 - addons/account_payment/models/account_move.py | 1 - .../static/src/js/payment_form.js | 25 ----- .../tests/test_payment_flows.py | 38 ++++---- .../views/payment_form_templates.xml | 11 --- addons/payment/controllers/portal.py | 41 ++++++++- .../static/src/js/express_checkout_form.js | 9 +- addons/payment/static/src/js/payment_form.js | 16 +++- addons/payment/tests/http_common.py | 7 +- addons/payment/tests/test_flows.py | 18 +++- .../payment/tests/test_multicompany_flows.py | 4 +- addons/payment/wizards/payment_link_wizard.py | 4 +- .../wizards/payment_link_wizard_views.xml | 1 - .../controllers/payment_portal.py | 14 +-- .../tests/online_payment_common.py | 3 - addons/sale/__manifest__.py | 2 - addons/sale/controllers/portal.py | 51 ++++------ addons/sale/models/sale_order.py | 1 - addons/sale/static/src/js/payment_form.js | 25 ----- addons/sale/tests/test_payment_flow.py | 92 ++++++++++--------- addons/sale/views/payment_form_templates.xml | 11 --- addons/website_payment/controllers/portal.py | 8 +- .../static/src/js/payment_form.js | 4 + addons/website_sale/controllers/main.py | 4 +- .../tests/test_website_sale_cart_payment.py | 17 +++- 27 files changed, 212 insertions(+), 252 deletions(-) delete mode 100644 addons/account_payment/static/src/js/payment_form.js delete mode 100644 addons/account_payment/views/payment_form_templates.xml delete mode 100644 addons/sale/static/src/js/payment_form.js delete mode 100644 addons/sale/views/payment_form_templates.xml diff --git a/addons/account_payment/__manifest__.py b/addons/account_payment/__manifest__.py index 1ee48e5717c..cdf165db8c4 100644 --- a/addons/account_payment/__manifest__.py +++ b/addons/account_payment/__manifest__.py @@ -19,7 +19,6 @@ 'views/account_move_views.xml', 'views/account_journal_views.xml', 'views/account_payment_views.xml', - 'views/payment_form_templates.xml', 'views/payment_provider_views.xml', 'views/payment_transaction_views.xml', @@ -28,11 +27,6 @@ 'wizards/payment_refund_wizard_views.xml', 'wizards/res_config_settings_views.xml', ], - 'assets': { - 'web.assets_frontend': [ - 'account_payment/static/src/js/payment_form.js', - ], - }, 'post_init_hook': 'post_init_hook', 'uninstall_hook': 'uninstall_hook', 'license': 'LGPL-3', diff --git a/addons/account_payment/controllers/payment.py b/addons/account_payment/controllers/payment.py index 7f5bab691fb..c6f9f52b2d6 100644 --- a/addons/account_payment/controllers/payment.py +++ b/addons/account_payment/controllers/payment.py @@ -30,11 +30,13 @@ class PaymentPortal(payment_portal.PaymentPortal): except AccessError: raise ValidationError(_("The access token is invalid.")) - kwargs['reference_prefix'] = None # Allow the reference to be computed based on the invoice logged_in = not request.env.user._is_public() - partner = request.env.user.partner_id if logged_in else invoice_sudo.partner_id - kwargs['partner_id'] = partner.id - kwargs.pop('custom_create_values', None) # Don't allow passing arbitrary create values + partner_sudo = request.env.user.partner_id if logged_in else invoice_sudo.partner_id + self._validate_transaction_kwargs(kwargs) + kwargs.update({ + 'currency_id': invoice_sudo.currency_id.id, + 'partner_id': partner_sudo.id, + }) # Inject the create values taken from the invoice into the kwargs. tx_sudo = self._create_transaction( custom_create_values={'invoice_ids': [Command.set([invoice_id])]}, **kwargs, ) @@ -47,9 +49,6 @@ class PaymentPortal(payment_portal.PaymentPortal): def payment_pay(self, *args, amount=None, invoice_id=None, access_token=None, **kwargs): """ Override of `payment` to replace the missing transaction values by that of the invoice. - This is necessary for the reconciliation as all transaction values, excepted the amount, - need to match exactly that of the invoice. - :param str amount: The (possibly partial) amount to pay used to check the access token. :param str invoice_id: The invoice for which a payment id made, as an `account.move` id. :param str access_token: The access token used to authenticate the partner. @@ -73,7 +72,11 @@ class PaymentPortal(payment_portal.PaymentPortal): raise ValidationError(_("The provided parameters are invalid.")) kwargs.update({ + # To display on the payment form; will be later overwritten when creating the tx. + 'reference': invoice_sudo.name, + # To fix the currency if incorrect and avoid mismatches when creating the tx. 'currency_id': invoice_sudo.currency_id.id, + # To fix the partner if incorrect and avoid mismatches when creating the tx. 'partner_id': invoice_sudo.partner_id.id, 'company_id': invoice_sudo.company_id.id, 'invoice_id': invoice_id, @@ -81,38 +84,27 @@ class PaymentPortal(payment_portal.PaymentPortal): return super().payment_pay(*args, amount=amount, access_token=access_token, **kwargs) def _get_extra_payment_form_values(self, invoice_id=None, **kwargs): - """ Override of `payment` to add the invoice id to the payment form values. + """ Override of `payment` to reroute the payment flow to the portal view of the invoice. - :param int invoice_id: The invoice for which a payment id made, as an `account.move` id. + :param str invoice_id: The invoice for which a payment id made, as an `account.move` id. :param dict kwargs: Optional data. This parameter is not used here. :return: The extended rendering context values. :rtype: dict """ form_values = super()._get_extra_payment_form_values(invoice_id=invoice_id, **kwargs) if invoice_id: - form_values['invoice_id'] = invoice_id + invoice_id = self._cast_as_int(invoice_id) + invoice_sudo = request.env['account.move'].sudo().browse(invoice_id) # Interrupt the payment flow if the invoice has been canceled. - invoice_sudo = request.env['account.move'].sudo().browse(invoice_id) if invoice_sudo.state == 'cancel': form_values['amount'] = 0.0 + # Reroute the next steps of the payment flow to the portal view of the invoice. + form_values.update({ + 'transaction_route': f'/invoice/transaction/{invoice_id}', + 'landing_route': f'{invoice_sudo.access_url}' + f'?access_token={invoice_sudo._portal_ensure_token()}', + 'access_token': invoice_sudo.access_token, + }) return form_values - - def _create_transaction(self, *args, invoice_id=None, custom_create_values=None, **kwargs): - """ Override of `payment` to add the invoice id in the custom create values. - - :param int invoice_id: The invoice 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: Optional data. This parameter is not used here. - :return: The result of the parent method. - :rtype: recordset of `payment.transaction` - """ - if invoice_id: - if custom_create_values is None: - custom_create_values = {} - custom_create_values['invoice_ids'] = [Command.set([int(invoice_id)])] - - return super()._create_transaction( - *args, invoice_id=invoice_id, custom_create_values=custom_create_values, **kwargs - ) diff --git a/addons/account_payment/controllers/portal.py b/addons/account_payment/controllers/portal.py index 218d361ef1e..db5c1588145 100644 --- a/addons/account_payment/controllers/portal.py +++ b/addons/account_payment/controllers/portal.py @@ -4,7 +4,6 @@ from odoo.http import request from odoo.addons.account.controllers import portal from odoo.addons.payment.controllers.portal import PaymentPortal -from odoo.addons.portal.controllers.portal import _build_url_w_params class PortalAccount(portal.PortalAccount, PaymentPortal): diff --git a/addons/account_payment/models/account_move.py b/addons/account_payment/models/account_move.py index 3ad2a0cdba3..9267a31348f 100644 --- a/addons/account_payment/models/account_move.py +++ b/addons/account_payment/models/account_move.py @@ -97,7 +97,6 @@ class AccountMove(models.Model): def _get_default_payment_link_values(self): self.ensure_one() return { - 'description': self.payment_reference, 'amount': self.amount_residual, 'currency_id': self.currency_id.id, 'partner_id': self.partner_id.id, diff --git a/addons/account_payment/static/src/js/payment_form.js b/addons/account_payment/static/src/js/payment_form.js deleted file mode 100644 index fcb32f1468c..00000000000 --- a/addons/account_payment/static/src/js/payment_form.js +++ /dev/null @@ -1,25 +0,0 @@ -/** @odoo-module **/ - -import paymentForm from '@payment/js/payment_form'; - -paymentForm.include({ - - // #=== PAYMENT FLOW ===# - - /** - * Add `invoice_id` to the params for the RPC to the transaction route. - * - * @override method from @payment/js/payment_form - * @private - * @return {object} The extended transaction route params. - */ - _prepareTransactionRouteParams() { - const transactionRouteParams = this._super(...arguments); - return { - ...transactionRouteParams, - 'invoice_id': this.paymentContext['invoiceId'] - ? parseInt(this.paymentContext['invoiceId']) : null, - }; - }, - -}); diff --git a/addons/account_payment/tests/test_payment_flows.py b/addons/account_payment/tests/test_payment_flows.py index f55b5bf0a17..67f64e85b79 100644 --- a/addons/account_payment/tests/test_payment_flows.py +++ b/addons/account_payment/tests/test_payment_flows.py @@ -1,6 +1,6 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo.tests import tagged +from odoo.tests import tagged, JsonRpcException from odoo.tools import mute_logger from odoo.addons.payment.tests.http_common import PaymentHttpCommon @@ -17,33 +17,35 @@ class TestFlows(AccountPaymentCommon, PaymentHttpCommon): route_values = self._prepare_pay_values() route_values['invoice_id'] = self.invoice.id tx_context = self._get_portal_pay_context(**route_values) - self.assertEqual(tx_context['invoice_id'], self.invoice.id) - # payment/transaction - route_values = { - k: tx_context[k] - for k in [ - 'amount', - 'currency_id', - 'reference_prefix', - 'partner_id', - 'access_token', - 'landing_route', - 'invoice_id', - ] - } - route_values.update({ + # /invoice/transaction/ + tx_route_values = { 'provider_id': self.provider.id, 'payment_method_id': self.payment_method_id, 'token_id': None, + 'amount': tx_context['amount'], 'flow': 'direct', 'tokenization_requested': False, - }) + 'landing_route': tx_context['landing_route'], + 'access_token': tx_context['access_token'], + } with mute_logger('odoo.addons.payment.models.payment_transaction'): - processing_values = self._get_processing_values(**route_values) + processing_values = self._get_processing_values( + tx_route=tx_context['transaction_route'], **tx_route_values + ) tx_sudo = self._get_tx(processing_values['reference']) # Note: strangely, the check # self.assertEqual(tx_sudo.invoice_ids, invoice) # doesn't work, and cache invalidation doesn't work either. self.invoice.invalidate_recordset(['transaction_ids']) self.assertEqual(self.invoice.transaction_ids, tx_sudo) + + @mute_logger('odoo.http') + def test_transaction_route_rejects_unexpected_kwarg(self): + url = self._build_url(f'/invoice/transaction/{self.invoice.id}/') + route_kwargs = { + 'access_token': self.invoice._portal_ensure_token(), + 'partner_id': self.partner.id, # This should be rejected. + } + with self.assertRaises(JsonRpcException, msg='odoo.exceptions.ValidationError'): + self.make_jsonrpc_request(url, route_kwargs) diff --git a/addons/account_payment/views/payment_form_templates.xml b/addons/account_payment/views/payment_form_templates.xml deleted file mode 100644 index 61bb8a455c3..00000000000 --- a/addons/account_payment/views/payment_form_templates.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - diff --git a/addons/payment/controllers/portal.py b/addons/payment/controllers/portal.py index 1deb7084710..ae4d2fd036e 100644 --- a/addons/payment/controllers/portal.py +++ b/addons/payment/controllers/portal.py @@ -260,7 +260,7 @@ class PaymentPortal(portal.CustomerPortal): if not payment_utils.check_access_token(access_token, partner_id, amount, currency_id): raise ValidationError(_("The access token is invalid.")) - kwargs.pop('custom_create_values', None) # Don't allow passing arbitrary create values + self._validate_transaction_kwargs(kwargs, additional_allowed_keys=('reference_prefix',)) tx_sudo = self._create_transaction( amount=amount, currency_id=currency_id, partner_id=partner_id, **kwargs ) @@ -268,8 +268,8 @@ class PaymentPortal(portal.CustomerPortal): return tx_sudo._get_processing_values() def _create_transaction( - self, provider_id, payment_method_id, token_id, reference_prefix, amount, currency_id, - partner_id, flow, tokenization_requested, landing_route, is_validation=False, + self, provider_id, payment_method_id, token_id, amount, currency_id, partner_id, flow, + tokenization_requested, landing_route, reference_prefix=None, is_validation=False, custom_create_values=None, **kwargs ): """ Create a draft transaction based on the payment context and return it. @@ -278,7 +278,6 @@ class PaymentPortal(portal.CustomerPortal): `payment.provider` id. :param int|None payment_method_id: The payment method, if any, as a `payment.method` id. :param int|None token_id: The token, if any, as a `payment.token` id. - :param str reference_prefix: The custom prefix to compute the full reference. :param float|None amount: The amount to pay, or `None` if in a validation operation. :param int|None currency_id: The currency of the amount, as a `res.currency` id, or `None` if in a validation operation. @@ -286,6 +285,7 @@ class PaymentPortal(portal.CustomerPortal): :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 landing_route: The route the user is redirected to after the transaction. + :param str reference_prefix: The custom prefix to compute the full reference. :param bool is_validation: Whether the operation is a validation. :param dict custom_create_values: Additional create values overwriting the default ones. :param dict kwargs: Locally unused data passed to `_is_tokenization_required` and @@ -462,3 +462,36 @@ class PaymentPortal(portal.CustomerPortal): :rtype: str """ return not partner.company_id or partner.company_id == document_company + + @staticmethod + def _validate_transaction_kwargs(kwargs, additional_allowed_keys=()): + """ Verify that the keys of a transaction route's kwargs are all whitelisted. + + The whitelist consists of all the keys that are expected to be passed to a transaction + route, plus optional contextually allowed keys. + + This method must be called in all transaction routes to ensure that no undesired kwarg can + be passed as param and then injected in the create values of the transaction. + + :param dict kwargs: The transaction route's kwargs to verify. + :param tuple additional_allowed_keys: The keys of kwargs that are contextually allowed. + :return: None + :raise ValidationError: If some kwargs keys are rejected. + """ + whitelist = { + 'provider_id', + 'payment_method_id', + 'token_id', + 'amount', + 'flow', + 'tokenization_requested', + 'landing_route', + 'is_validation', + 'csrf_token', + } + whitelist.update(additional_allowed_keys) + rejected_keys = set(kwargs.keys()) - whitelist + if rejected_keys: + raise ValidationError( + _("The following kwargs are not whitelisted: %s", ', '.join(rejected_keys)) + ) diff --git a/addons/payment/static/src/js/express_checkout_form.js b/addons/payment/static/src/js/express_checkout_form.js index 39e588d41f1..4297ed1062a 100644 --- a/addons/payment/static/src/js/express_checkout_form.js +++ b/addons/payment/static/src/js/express_checkout_form.js @@ -58,12 +58,9 @@ publicWidget.registry.PaymentExpressCheckoutForm = publicWidget.Widget.extend({ */ _prepareTransactionRouteParams(providerId) { return { - 'payment_option_id': parseInt(providerId), - 'reference_prefix': this.paymentContext['referencePrefix'] && - this.paymentContext['referencePrefix'].toString(), - 'currency_id': this.paymentContext['currencyId'] && - parseInt(this.paymentContext['currencyId']), - 'partner_id': parseInt(this.paymentContext['partnerId']), + 'provider_id': parseInt(providerId), + 'payment_method_id': 1, // TODO VCR + 'token_id': null, 'flow': 'direct', 'tokenization_requested': false, 'landing_route': this.paymentContext['landingRoute'], diff --git a/addons/payment/static/src/js/payment_form.js b/addons/payment/static/src/js/payment_form.js index 8b2f5a9e03c..ea7d4c59c89 100644 --- a/addons/payment/static/src/js/payment_form.js +++ b/addons/payment/static/src/js/payment_form.js @@ -407,16 +407,12 @@ publicWidget.registry.PaymentForm = publicWidget.Widget.extend({ * @return {object} The transaction route params. */ _prepareTransactionRouteParams() { - return { + let transactionRouteParams = { 'provider_id': this.paymentContext.providerId, 'payment_method_id': this.paymentContext.paymentMethodId ?? null, 'token_id': this.paymentContext.tokenId ?? null, - 'reference_prefix': this.paymentContext['referencePrefix']?.toString() ?? null, 'amount': this.paymentContext['amount'] !== undefined ? parseFloat(this.paymentContext['amount']) : null, - 'currency_id': this.paymentContext['currencyId'] - ? parseInt(this.paymentContext['currencyId']) : null, - 'partner_id': parseInt(this.paymentContext['partnerId']), 'flow': this.paymentContext['flow'], 'tokenization_requested': this.paymentContext['tokenizationRequested'], 'landing_route': this.paymentContext['landingRoute'], @@ -424,6 +420,16 @@ publicWidget.registry.PaymentForm = publicWidget.Widget.extend({ 'access_token': this.paymentContext['accessToken'], 'csrf_token': odoo.csrf_token, }; + // Generic payment flows (i.e., that are not attached to a document) require extra params. + if (this.paymentContext['transactionRoute'] === '/payment/transaction') { + Object.assign(transactionRouteParams, { + 'currency_id': this.paymentContext['currencyId'] + ? parseInt(this.paymentContext['currencyId']) : null, + 'partner_id': parseInt(this.paymentContext['partnerId']), + 'reference_prefix': this.paymentContext['referencePrefix']?.toString(), + }); + } + return transactionRouteParams; }, /** diff --git a/addons/payment/tests/http_common.py b/addons/payment/tests/http_common.py index 2d341b3c0c3..dab6168e9b1 100644 --- a/addons/payment/tests/http_common.py +++ b/addons/payment/tests/http_common.py @@ -205,20 +205,19 @@ class PaymentHttpCommon(PaymentCommon, HttpCase): 'access_token': self._generate_test_access_token( self.partner.id, self.amount, self.currency.id ), - 'reference_prefix': 'test', 'tokenization_requested': True, 'landing_route': 'Test', + 'reference_prefix': 'test', 'is_validation': False, 'flow': flow, } - def _portal_transaction(self, **route_kwargs): + def _portal_transaction(self, tx_route='/payment/transaction', **route_kwargs): """/payment/transaction feedback :return: The response to the json request """ - uri = '/payment/transaction' - url = self._build_url(uri) + url = self._build_url(tx_route) return self.make_jsonrpc_request(url, route_kwargs) def _get_processing_values(self, **route_kwargs): diff --git a/addons/payment/tests/test_flows.py b/addons/payment/tests/test_flows.py index 993bce8be33..af607ace5c1 100644 --- a/addons/payment/tests/test_flows.py +++ b/addons/payment/tests/test_flows.py @@ -37,10 +37,10 @@ class TestFlows(PaymentHttpCommon): for k in [ 'amount', 'currency_id', - 'reference_prefix', 'partner_id', - 'access_token', 'landing_route', + 'reference_prefix', + 'access_token', ] } route_values.update({ @@ -180,8 +180,8 @@ class TestFlows(PaymentHttpCommon): 'access_token': payment_context['access_token'], 'flow': flow, 'tokenization_requested': True, - 'reference_prefix': payment_context['reference_prefix'], 'landing_route': payment_context['landing_route'], + 'reference_prefix': payment_context['reference_prefix'], 'is_validation': True, } with mute_logger('odoo.addons.payment.models.payment_transaction'): @@ -277,12 +277,13 @@ class TestFlows(PaymentHttpCommon): def test_transaction_wrong_flow(self): transaction_values = self._prepare_pay_values() + transaction_values.pop('reference') transaction_values.update({ 'flow': 'this flow does not exist', 'payment_option_id': self.provider.id, 'tokenization_requested': False, - 'reference_prefix': 'whatever', 'landing_route': 'whatever', + 'reference_prefix': 'whatever', }) # Transaction step with a wrong flow --> UserError with mute_logger("odoo.http"), self.assertRaises( @@ -291,6 +292,15 @@ class TestFlows(PaymentHttpCommon): ): self._portal_transaction(**transaction_values) + @mute_logger('odoo.http') + def test_transaction_route_rejects_unexpected_kwarg(self): + route_kwargs = { + **self._prepare_pay_values(), + 'custom_create_values': 'whatever', # This should be rejected. + } + with self.assertRaises(JsonRpcException, msg='odoo.exceptions.ValidationError'): + self._portal_transaction(**route_kwargs) + def test_transaction_wrong_token(self): route_values = self._prepare_pay_values() route_values['access_token'] = "abcde" diff --git a/addons/payment/tests/test_multicompany_flows.py b/addons/payment/tests/test_multicompany_flows.py index ebff47439a9..9964c072ef9 100644 --- a/addons/payment/tests/test_multicompany_flows.py +++ b/addons/payment/tests/test_multicompany_flows.py @@ -64,10 +64,10 @@ class TestMultiCompanyFlows(PaymentHttpCommon): for k in [ 'amount', 'currency_id', - 'reference_prefix', 'partner_id', - 'access_token', 'landing_route', + 'reference_prefix', + 'access_token', ] } validation_values.update({ diff --git a/addons/payment/wizards/payment_link_wizard.py b/addons/payment/wizards/payment_link_wizard.py index 464f3091ea2..983ff651a33 100644 --- a/addons/payment/wizards/payment_link_wizard.py +++ b/addons/payment/wizards/payment_link_wizard.py @@ -30,7 +30,6 @@ class PaymentLinkWizard(models.TransientModel): currency_id = fields.Many2one('res.currency') partner_id = fields.Many2one('res.partner') partner_email = fields.Char(related='partner_id.email') - description = fields.Char("Payment Ref") link = fields.Char(string="Payment Link", compute='_compute_link') company_id = fields.Many2one('res.company', compute='_compute_company_id') warning_message = fields.Char(compute='_compute_warning_message') @@ -58,13 +57,12 @@ class PaymentLinkWizard(models.TransientModel): self.partner_id.id, self.amount, self.currency_id.id ) - @api.depends('description', 'amount', 'currency_id', 'partner_id', 'company_id') + @api.depends('amount', 'currency_id', 'partner_id', 'company_id') def _compute_link(self): for payment_link in self: related_document = self.env[payment_link.res_model].browse(payment_link.res_id) base_url = related_document.get_base_url() # Don't generate links for the wrong website url_params = { - 'reference': urls.url_quote(payment_link.description), 'amount': self.amount, 'access_token': self._get_access_token(), **self._get_additional_link_values(), diff --git a/addons/payment/wizards/payment_link_wizard_views.xml b/addons/payment/wizards/payment_link_wizard_views.xml index 656130ab694..1cce5dcaf94 100644 --- a/addons/payment/wizards/payment_link_wizard_views.xml +++ b/addons/payment/wizards/payment_link_wizard_views.xml @@ -19,7 +19,6 @@ - diff --git a/addons/pos_online_payment/controllers/payment_portal.py b/addons/pos_online_payment/controllers/payment_portal.py index 16fd87dc59f..0e10636386b 100644 --- a/addons/pos_online_payment/controllers/payment_portal.py +++ b/addons/pos_online_payment/controllers/payment_portal.py @@ -178,6 +178,7 @@ class PaymentPortal(payment_portal.PaymentPortal): if not partner_sudo: return self._redirect_login() + self._validate_transaction_kwargs(kwargs) if kwargs.get('is_validation'): raise UserError( _("A validation payment cannot be used for a Point of Sale online payment.")) @@ -185,12 +186,13 @@ class PaymentPortal(payment_portal.PaymentPortal): if 'partner_id' in kwargs and kwargs['partner_id'] != partner_sudo.id: raise UserError( _("The provided partner_id is different than expected.")) - - # Don't allow passing arbitrary create values and avoid tokenization for - # the public user. - kwargs['custom_create_values'] = { - 'pos_order_id': pos_order_sudo.id - } + # Avoid tokenization for the public user. + kwargs.update({ + 'partner_id': partner_sudo.id, + 'custom_create_values': { + 'pos_order_id': pos_order_sudo.id, + }, + }) if not logged_in: if kwargs.get('tokenization_requested') or kwargs.get('flow') == 'token': raise UserError( diff --git a/addons/pos_online_payment/tests/online_payment_common.py b/addons/pos_online_payment/tests/online_payment_common.py index f2a6568c3e9..bb3bc97319e 100644 --- a/addons/pos_online_payment/tests/online_payment_common.py +++ b/addons/pos_online_payment/tests/online_payment_common.py @@ -37,9 +37,6 @@ class OnlinePaymentCommon(PaymentHttpCommon): k: payment_context[k] for k in [ 'amount', - 'currency_id', - 'reference_prefix', - 'partner_id', 'access_token', 'landing_route', ] diff --git a/addons/sale/__manifest__.py b/addons/sale/__manifest__.py index c20d4837b09..011594b4e4f 100644 --- a/addons/sale/__manifest__.py +++ b/addons/sale/__manifest__.py @@ -46,7 +46,6 @@ This module contains all the common features of Sales Management and eCommerce. 'views/account_views.xml', 'views/crm_team_views.xml', 'views/mail_activity_views.xml', - 'views/payment_form_templates.xml', 'views/payment_views.xml', 'views/product_document_views.xml', 'views/product_packaging_views.xml', @@ -79,7 +78,6 @@ This module contains all the common features of Sales Management and eCommerce. 'sale/static/src/js/sale_portal_sidebar.js', 'sale/static/src/js/sale_portal_prepayment.js', 'sale/static/src/js/sale_portal.js', - 'sale/static/src/js/payment_form.js', ], 'web.assets_tests': [ 'sale/static/tests/tours/**/*', diff --git a/addons/sale/controllers/portal.py b/addons/sale/controllers/portal.py index 4977e897e75..f7aa7596253 100644 --- a/addons/sale/controllers/portal.py +++ b/addons/sale/controllers/portal.py @@ -360,12 +360,12 @@ class PaymentPortal(payment_portal.PaymentPortal): logged_in = not request.env.user._is_public() partner_sudo = request.env.user.partner_id if logged_in else order_sudo.partner_invoice_id + self._validate_transaction_kwargs(kwargs) kwargs.update({ - 'reference_prefix': None, # Allow the reference to be computed based on the order 'partner_id': partner_sudo.id, + 'currency_id': order_sudo.currency_id.id, 'sale_order_id': order_id, # Include the SO to allow Subscriptions tokenizing the tx }) - kwargs.pop('custom_create_values', None) # Don't allow passing arbitrary create values tx_sudo = self._create_transaction( custom_create_values={'sale_order_ids': [Command.set([order_id])]}, **kwargs, ) @@ -376,10 +376,8 @@ class PaymentPortal(payment_portal.PaymentPortal): @http.route() def payment_pay(self, *args, amount=None, sale_order_id=None, access_token=None, **kwargs): - """ Override of payment to replace the missing transaction values by that of the sale order. - - This is necessary for the reconciliation as all transaction values, excepted the amount, - need to match exactly that of the sale order. + """ Override of `payment` to replace the missing transaction values by that of the sales + order. :param str amount: The (possibly partial) amount to pay used to check the access token :param str sale_order_id: The sale order for which a payment id made, as a `sale.order` id @@ -404,7 +402,11 @@ class PaymentPortal(payment_portal.PaymentPortal): raise ValidationError(_("The provided parameters are invalid.")) kwargs.update({ + # To display on the payment form; will be later overwritten when creating the tx. + 'reference': order_sudo.name, + # To fix the currency if incorrect and avoid mismatches when creating the tx. 'currency_id': order_sudo.currency_id.id, + # To fix the partner if incorrect and avoid mismatches when creating the tx. 'partner_id': order_sudo.partner_invoice_id.id, 'company_id': order_sudo.company_id.id, 'sale_order_id': sale_order_id, @@ -412,38 +414,25 @@ class PaymentPortal(payment_portal.PaymentPortal): return super().payment_pay(*args, amount=amount, access_token=access_token, **kwargs) def _get_extra_payment_form_values(self, sale_order_id=None, **kwargs): - """ Override of `payment` to add the sale order id to the payment form values. + """ Override of `payment` to reroute the payment flow to the portal view of the sales order. - :param int sale_order_id: The sale order for which a payment id made, as a `sale.order` id - :return: The extended rendering context values + :param str sale_order_id: The sale order for which a payment is made, as a `sale.order` id. + :return: The extended rendering context values. :rtype: dict """ form_values = super()._get_extra_payment_form_values(sale_order_id=sale_order_id, **kwargs) if sale_order_id: - form_values['sale_order_id'] = sale_order_id + sale_order_id = self._cast_as_int(sale_order_id) + order_sudo = request.env['sale.order'].sudo().browse(sale_order_id) # Interrupt the payment flow if the sales order has been canceled. - order_sudo = request.env['sale.order'].sudo().browse(sale_order_id) if order_sudo.state == 'cancel': form_values['amount'] = 0.0 + + # Reroute the next steps of the payment flow to the portal view of the sales order. + form_values.update({ + 'transaction_route': order_sudo.get_portal_url(suffix='/transaction'), + 'landing_route': order_sudo.get_portal_url(), + 'access_token': order_sudo.access_token, + }) return form_values - - def _create_transaction(self, *args, sale_order_id=None, custom_create_values=None, **kwargs): - """ Override of payment to add the sale order id in the custom create values. - - :param int sale_order_id: The sale order for which a payment id made, as a `sale.order` id - :param dict custom_create_values: Additional create values overwriting the default ones - :return: The result of the parent method - :rtype: recordset of `payment.transaction` - """ - if sale_order_id: - if custom_create_values is None: - custom_create_values = {} - # As this override is also called if the flow is initiated from sale or website_sale, we - # must avoid overriding whatever value these modules could have already set. - if 'sale_order_ids' not in custom_create_values: # We are in the payment module's flow - custom_create_values['sale_order_ids'] = [Command.set([int(sale_order_id)])] - - return super()._create_transaction( - *args, sale_order_id=sale_order_id, custom_create_values=custom_create_values, **kwargs - ) diff --git a/addons/sale/models/sale_order.py b/addons/sale/models/sale_order.py index c0929d7979f..352877088e6 100644 --- a/addons/sale/models/sale_order.py +++ b/addons/sale/models/sale_order.py @@ -1569,7 +1569,6 @@ class SaleOrder(models.Model): amount = amount_max return { - 'description': self.name, 'currency_id': self.currency_id.id, 'partner_id': self.partner_invoice_id.id, 'amount': amount, diff --git a/addons/sale/static/src/js/payment_form.js b/addons/sale/static/src/js/payment_form.js deleted file mode 100644 index d33c8071e8f..00000000000 --- a/addons/sale/static/src/js/payment_form.js +++ /dev/null @@ -1,25 +0,0 @@ -/** @odoo-module **/ - -import paymentForm from '@payment/js/payment_form'; - -paymentForm.include({ - - // #=== PAYMENT FLOW ===# - - /** - * Add `sale_order_id` to the params for the RPC to the transaction route. - * - * @override method from @payment/js/payment_form - * @private - * @return {object} The extended transaction route params. - */ - _prepareTransactionRouteParams() { - const transactionRouteParams = this._super(...arguments); - return { - ...transactionRouteParams, - 'sale_order_id': this.paymentContext['saleOrderId'] - ? parseInt(this.paymentContext['saleOrderId']) : undefined, - }; - }, - -}); diff --git a/addons/sale/tests/test_payment_flow.py b/addons/sale/tests/test_payment_flow.py index 71c14cfa25e..aa3d698f49a 100644 --- a/addons/sale/tests/test_payment_flow.py +++ b/addons/sale/tests/test_payment_flow.py @@ -2,7 +2,7 @@ from unittest.mock import ANY, patch from odoo.fields import Command -from odoo.tests import tagged +from odoo.tests import tagged, JsonRpcException from odoo.tools import mute_logger from odoo.addons.account_payment.tests.common import AccountPaymentCommon @@ -37,23 +37,22 @@ class TestSalePayment(AccountPaymentCommon, SaleCommon, PaymentHttpCommon): self.assertEqual(tx_context['currency_id'], self.sale_order.currency_id.id) self.assertEqual(tx_context['partner_id'], self.sale_order.partner_invoice_id.id) self.assertEqual(tx_context['amount'], self.sale_order.amount_total) - self.assertEqual(tx_context['sale_order_id'], self.sale_order.id) - route_values.update({ + # /my/orders//transaction/ + tx_route_values = { 'provider_id': self.provider.id, 'payment_method_id': self.payment_method_id, 'token_id': None, + 'amount': tx_context['amount'], 'flow': 'direct', 'tokenization_requested': False, - 'validation_route': False, - 'reference_prefix': None, # Force empty prefix to fallback on SO reference 'landing_route': tx_context['landing_route'], - 'amount': tx_context['amount'], - 'currency_id': tx_context['currency_id'], - }) - + 'access_token': tx_context['access_token'], + } with mute_logger('odoo.addons.payment.models.payment_transaction'): - processing_values = self._get_processing_values(**route_values) + processing_values = self._get_processing_values( + tx_route=tx_context['transaction_route'], **tx_route_values + ) tx_sudo = self._get_tx(processing_values['reference']) self.assertEqual(tx_sudo.sale_order_ids, self.sale_order) @@ -87,29 +86,29 @@ class TestSalePayment(AccountPaymentCommon, SaleCommon, PaymentHttpCommon): # test customized /payment/pay route with sale_order_id param # partial amount specified self.amount = self.sale_order.amount_total / 2.0 - route_values = self._prepare_pay_values() - route_values['sale_order_id'] = self.sale_order.id + pay_route_values = self._prepare_pay_values() + pay_route_values['sale_order_id'] = self.sale_order.id - tx_context = self._get_portal_pay_context(**route_values) + tx_context = self._get_portal_pay_context(**pay_route_values) - self.assertEqual(tx_context['reference_prefix'], self.reference) self.assertEqual(tx_context['currency_id'], self.sale_order.currency_id.id) self.assertEqual(tx_context['partner_id'], self.sale_order.partner_invoice_id.id) self.assertEqual(tx_context['amount'], self.amount) - self.assertEqual(tx_context['sale_order_id'], self.sale_order.id) - route_values.update({ + tx_route_values = { 'provider_id': self.provider.id, 'payment_method_id': self.payment_method_id, 'token_id': None, + 'amount': tx_context['amount'], 'flow': 'direct', 'tokenization_requested': False, - 'validation_route': False, - 'reference_prefix': tx_context['reference_prefix'], 'landing_route': tx_context['landing_route'], - }) + 'access_token': tx_context['access_token'], + } with mute_logger('odoo.addons.payment.models.payment_transaction'): - processing_values = self._get_processing_values(**route_values) + processing_values = self._get_processing_values( + tx_route=tx_context['transaction_route'], **tx_route_values + ) tx_sudo = self._get_tx(processing_values['reference']) self.assertEqual(tx_sudo.sale_order_ids, self.sale_order) @@ -117,7 +116,6 @@ class TestSalePayment(AccountPaymentCommon, SaleCommon, PaymentHttpCommon): self.assertEqual(tx_sudo.partner_id, self.sale_order.partner_invoice_id) self.assertEqual(tx_sudo.company_id, self.sale_order.company_id) self.assertEqual(tx_sudo.currency_id, self.sale_order.currency_id) - self.assertEqual(tx_sudo.reference, self.reference) self.assertEqual(tx_sudo.sale_order_ids.transaction_ids, tx_sudo) tx_sudo._set_done() @@ -126,29 +124,29 @@ class TestSalePayment(AccountPaymentCommon, SaleCommon, PaymentHttpCommon): self.assertEqual(self.sale_order.state, 'draft') # Only a partial amount was paid # Pay the remaining amount - route_values = self._prepare_pay_values() - route_values['sale_order_id'] = self.sale_order.id + pay_route_values = self._prepare_pay_values() + pay_route_values['sale_order_id'] = self.sale_order.id - tx_context = self._get_portal_pay_context(**route_values) + tx_context = self._get_portal_pay_context(**pay_route_values) - self.assertEqual(tx_context['reference_prefix'], self.reference) self.assertEqual(tx_context['currency_id'], self.sale_order.currency_id.id) self.assertEqual(tx_context['partner_id'], self.sale_order.partner_invoice_id.id) self.assertEqual(tx_context['amount'], self.amount) - self.assertEqual(tx_context['sale_order_id'], self.sale_order.id) - route_values.update({ + tx_route_values = { 'provider_id': self.provider.id, 'payment_method_id': self.payment_method_id, 'token_id': None, + 'amount': tx_context['amount'], 'flow': 'direct', 'tokenization_requested': False, - 'validation_route': False, - 'reference_prefix': tx_context['reference_prefix'], 'landing_route': tx_context['landing_route'], - }) + 'access_token': tx_context['access_token'], + } with mute_logger('odoo.addons.payment.models.payment_transaction'): - processing_values = self._get_processing_values(**route_values) + processing_values = self._get_processing_values( + tx_route=tx_context['transaction_route'], **tx_route_values + ) tx2_sudo = self._get_tx(processing_values['reference']) self.assertEqual(tx2_sudo.sale_order_ids, self.sale_order) @@ -157,10 +155,6 @@ class TestSalePayment(AccountPaymentCommon, SaleCommon, PaymentHttpCommon): self.assertEqual(tx2_sudo.company_id, self.sale_order.company_id) self.assertEqual(tx2_sudo.currency_id, self.sale_order.currency_id) - # We are paying a second time with the same reference (prefix) - # a suffix is added to respect unique reference constraint - reference = self.reference + "-1" - self.assertEqual(tx2_sudo.reference, reference) self.assertEqual(self.sale_order.state, 'draft') self.assertEqual(self.sale_order.transaction_ids, tx_sudo + tx2_sudo) @@ -173,23 +167,25 @@ class TestSalePayment(AccountPaymentCommon, SaleCommon, PaymentHttpCommon): self.product.invoice_policy = 'delivery' self.amount = self.sale_order.amount_total / 2.0 - route_values = self._prepare_pay_values() - route_values['sale_order_id'] = self.sale_order.id + pay_route_values = self._prepare_pay_values() + pay_route_values['sale_order_id'] = self.sale_order.id - tx_context = self._get_portal_pay_context(**route_values) + tx_context = self._get_portal_pay_context(**pay_route_values) - route_values.update({ + tx_route_values = { 'provider_id': self.provider.id, 'payment_method_id': self.payment_method_id, 'token_id': None, + 'amount': tx_context['amount'], 'flow': 'direct', 'tokenization_requested': False, - 'validation_route': False, - 'reference_prefix': tx_context['reference_prefix'], 'landing_route': tx_context['landing_route'], - }) + 'access_token': tx_context['access_token'], + } with mute_logger('odoo.addons.payment.models.payment_transaction'): - processing_values = self._get_processing_values(**route_values) + processing_values = self._get_processing_values( + tx_route=tx_context['transaction_route'], **tx_route_values + ) tx_sudo = self._get_tx(processing_values['reference']) tx_sudo._set_done() @@ -388,3 +384,13 @@ class TestSalePayment(AccountPaymentCommon, SaleCommon, PaymentHttpCommon): invoice = self.sale_order.invoice_ids self.assertTrue(len(invoice) == 1) self.assertTrue(invoice.line_ids[0].is_downpayment) + + @mute_logger('odoo.http') + def test_transaction_route_rejects_unexpected_kwarg(self): + url = self._build_url(f'/my/orders/{self.sale_order.id}/transaction') + route_kwargs = { + 'access_token': self.sale_order._portal_ensure_token(), + 'partner_id': self.partner.id, # This should be rejected. + } + with self.assertRaises(JsonRpcException, msg='odoo.exceptions.ValidationError'): + self.make_jsonrpc_request(url, route_kwargs) diff --git a/addons/sale/views/payment_form_templates.xml b/addons/sale/views/payment_form_templates.xml deleted file mode 100644 index 62b8b1d1002..00000000000 --- a/addons/sale/views/payment_form_templates.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - diff --git a/addons/website_payment/controllers/portal.py b/addons/website_payment/controllers/portal.py index 769abfdea9c..a9d19db2838 100644 --- a/addons/website_payment/controllers/portal.py +++ b/addons/website_payment/controllers/portal.py @@ -51,13 +51,11 @@ class PaymentPortal(payment_portal.PaymentPortal): else: partner_id = request.env.user.partner_id.id - # Don't allow passing arbitrary create values and avoid tokenization for - # the public user. + self._validate_transaction_kwargs(kwargs, additional_allowed_keys=( + 'donation_comment', 'donation_recipient_email', 'partner_details', 'reference_prefix' + )) if use_public_partner: kwargs['custom_create_values'] = {'tokenize': False} - else: - kwargs.pop('custom_create_values', None) - tx_sudo = self._create_transaction( amount=amount, currency_id=currency_id, partner_id=partner_id, **kwargs ) diff --git a/addons/website_payment/static/src/js/payment_form.js b/addons/website_payment/static/src/js/payment_form.js index 796ee535888..76df1329cb7 100644 --- a/addons/website_payment/static/src/js/payment_form.js +++ b/addons/website_payment/static/src/js/payment_form.js @@ -105,6 +105,10 @@ PaymentForm.include({ const transactionRouteParams = this._super(...arguments); return $('.o_donation_payment_form').length ? { ...transactionRouteParams, + 'partner_id': parseInt(this.paymentContext['partnerId']), + 'currency_id': this.paymentContext['currencyId'] + ? parseInt(this.paymentContext['currencyId']) : null, + 'reference_prefix':this.paymentContext['referencePrefix']?.toString(), 'partner_details': { 'name': this.$('input[name="name"]').val(), 'email': this.$('input[name="email"]').val(), diff --git a/addons/website_sale/controllers/main.py b/addons/website_sale/controllers/main.py index 63d4ca159a7..4c4d32e009d 100644 --- a/addons/website_sale/controllers/main.py +++ b/addons/website_sale/controllers/main.py @@ -1861,12 +1861,12 @@ class PaymentPortal(payment_portal.PaymentPortal): order_sudo._check_cart_is_ready_to_be_paid() + self._validate_transaction_kwargs(kwargs) kwargs.update({ - 'reference_prefix': None, # Allow the reference to be computed based on the order 'partner_id': order_sudo.partner_invoice_id.id, + 'currency_id': order_sudo.currency_id.id, 'sale_order_id': order_id, # Include the SO to allow Subscriptions to tokenize the tx }) - kwargs.pop('custom_create_values', None) # Don't allow passing arbitrary create values if not kwargs.get('amount'): kwargs['amount'] = order_sudo.amount_total diff --git a/addons/website_sale/tests/test_website_sale_cart_payment.py b/addons/website_sale/tests/test_website_sale_cart_payment.py index dba93a6345d..fc8ca8731af 100644 --- a/addons/website_sale/tests/test_website_sale_cart_payment.py +++ b/addons/website_sale/tests/test_website_sale_cart_payment.py @@ -1,14 +1,15 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo.models import Command -from odoo.tests.common import tagged +from odoo.tests.common import tagged, JsonRpcException +from odoo.tools import mute_logger -from odoo.addons.payment.tests.common import PaymentCommon +from odoo.addons.payment.tests.http_common import PaymentHttpCommon from odoo.addons.website.tools import MockRequest @tagged('post_install', '-at_install') -class WebsiteSaleCartPayment(PaymentCommon): +class WebsiteSaleCartPayment(PaymentHttpCommon): @classmethod def setUpClass(cls): @@ -53,3 +54,13 @@ class WebsiteSaleCartPayment(PaymentCommon): msg=f"The transaction state '{paid_order_tx_state}' should prevent retrieving " f"the linked order.", ) + + @mute_logger('odoo.http') + def test_transaction_route_rejects_unexpected_kwarg(self): + url = self._build_url(f'/shop/payment/transaction/{self.order.id}') + route_kwargs = { + 'access_token': self.order._portal_ensure_token(), + 'partner_id': self.partner.id, # This should be rejected. + } + with self.assertRaises(JsonRpcException, msg='odoo.exceptions.ValidationError'): + self.make_jsonrpc_request(url, route_kwargs)