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)