diff --git a/addons/account_payment/controllers/payment.py b/addons/account_payment/controllers/payment.py index c6f9f52b2d6..43ab7f001ec 100644 --- a/addons/account_payment/controllers/payment.py +++ b/addons/account_payment/controllers/payment.py @@ -83,18 +83,32 @@ 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): + def _get_extra_payment_form_values(self, invoice_id=None, access_token=None, **kwargs): """ Override of `payment` to reroute the payment flow to the portal view of the invoice. :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. + :param str access_token: The portal or payment access token, respectively if we are in a + portal or payment link flow. :return: The extended rendering context values. :rtype: dict """ - form_values = super()._get_extra_payment_form_values(invoice_id=invoice_id, **kwargs) + form_values = super()._get_extra_payment_form_values( + invoice_id=invoice_id, access_token=access_token, **kwargs + ) if invoice_id: invoice_id = self._cast_as_int(invoice_id) - invoice_sudo = request.env['account.move'].sudo().browse(invoice_id) + + try: # Check document access against what could be a portal access token. + invoice_sudo = self._document_check_access('account.move', invoice_id, access_token) + except AccessError: # It is a payment access token computed on the payment context. + if not payment_utils.check_access_token( + access_token, + kwargs.get('partner_id'), + kwargs.get('amount'), + kwargs.get('currency_id'), + ): + raise + invoice_sudo = request.env['account.move'].sudo().browse(invoice_id) # Interrupt the payment flow if the invoice has been canceled. if invoice_sudo.state == 'cancel': diff --git a/addons/account_payment/tests/test_payment_flows.py b/addons/account_payment/tests/test_payment_flows.py index 67f64e85b79..744df0db4cd 100644 --- a/addons/account_payment/tests/test_payment_flows.py +++ b/addons/account_payment/tests/test_payment_flows.py @@ -1,10 +1,15 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. +from unittest.mock import patch + +from odoo.exceptions import AccessError from odoo.tests import tagged, JsonRpcException from odoo.tools import mute_logger -from odoo.addons.payment.tests.http_common import PaymentHttpCommon +from odoo.addons.account_payment.controllers.payment import PaymentPortal from odoo.addons.account_payment.tests.common import AccountPaymentCommon +from odoo.addons.payment.tests.http_common import PaymentHttpCommon +from odoo.addons.portal.controllers.portal import CustomerPortal @tagged('post_install', '-at_install') @@ -40,6 +45,50 @@ class TestFlows(AccountPaymentCommon, PaymentHttpCommon): self.invoice.invalidate_recordset(['transaction_ids']) self.assertEqual(self.invoice.transaction_ids, tx_sudo) + def test_check_portal_access_token_before_rerouting_flow(self): + """ Test that access to the provided invoice is checked against the portal access token + before rerouting the payment flow. """ + payment_portal_controller = PaymentPortal() + + with patch.object(CustomerPortal, '_document_check_access') as mock: + payment_portal_controller._get_extra_payment_form_values() + self.assertEqual( + mock.call_count, 0, msg="No check should be made when invoice_id is not provided." + ) + + mock.reset_mock() + + payment_portal_controller._get_extra_payment_form_values( + invoice_id=self.invoice.id, access_token='whatever' + ) + self.assertEqual( + mock.call_count, 1, msg="The check should be made as invoice_id is provided." + ) + + def test_check_payment_access_token_before_rerouting_flow(self): + """ Test that access to the provided invoice is checked against the payment access token + before rerouting the payment flow. """ + payment_portal_controller = PaymentPortal() + + def _document_check_access_mock(*_args, **_kwargs): + raise AccessError('') + + with patch.object( + CustomerPortal, '_document_check_access', _document_check_access_mock + ), patch('odoo.addons.payment.utils.check_access_token') as check_payment_access_token_mock: + try: + payment_portal_controller._get_extra_payment_form_values( + invoice_id=self.invoice.id, access_token='whatever' + ) + except Exception: + pass # We don't care if it runs or not; we only count the calls. + self.assertEqual( + check_payment_access_token_mock.call_count, + 1, + msg="The access token should be checked again as a payment access token if the" + " check as a portal access token failed.", + ) + @mute_logger('odoo.http') def test_transaction_route_rejects_unexpected_kwarg(self): url = self._build_url(f'/invoice/transaction/{self.invoice.id}/') diff --git a/addons/payment/controllers/portal.py b/addons/payment/controllers/portal.py index ae4d2fd036e..7af7c39b852 100644 --- a/addons/payment/controllers/portal.py +++ b/addons/payment/controllers/portal.py @@ -154,7 +154,9 @@ class PaymentPortal(portal.CustomerPortal): **portal_page_values, **payment_form_values, **payment_context, - **self._get_extra_payment_form_values(**kwargs), + **self._get_extra_payment_form_values( + **payment_context, currency_id=currency.id, **kwargs + ), # Pass the payment context to allow overriding modules to check document access. } return request.render(self._get_payment_page_template_xmlid(**kwargs), rendering_context) diff --git a/addons/sale/controllers/portal.py b/addons/sale/controllers/portal.py index d3ee6f96d38..5206880e0d3 100644 --- a/addons/sale/controllers/portal.py +++ b/addons/sale/controllers/portal.py @@ -418,17 +418,32 @@ 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): + def _get_extra_payment_form_values(self, sale_order_id=None, access_token=None, **kwargs): """ Override of `payment` to reroute the payment flow to the portal view of the sales order. :param str sale_order_id: The sale order for which a payment is made, as a `sale.order` id. + :param str access_token: The portal or payment access token, respectively if we are in a + portal or payment link flow. :return: The extended rendering context values. :rtype: dict """ - form_values = super()._get_extra_payment_form_values(sale_order_id=sale_order_id, **kwargs) + form_values = super()._get_extra_payment_form_values( + sale_order_id=sale_order_id, access_token=access_token, **kwargs + ) if sale_order_id: sale_order_id = self._cast_as_int(sale_order_id) - order_sudo = request.env['sale.order'].sudo().browse(sale_order_id) + + try: # Check document access against what could be a portal access token. + order_sudo = self._document_check_access('sale.order', sale_order_id, access_token) + except AccessError: # It is a payment access token computed on the payment context. + if not payment_utils.check_access_token( + access_token, + kwargs.get('partner_id'), + kwargs.get('amount'), + kwargs.get('currency_id'), + ): + raise + order_sudo = request.env['sale.order'].sudo().browse(sale_order_id) # Interrupt the payment flow if the sales order has been canceled. if order_sudo.state == 'cancel': diff --git a/addons/sale/tests/test_payment_flow.py b/addons/sale/tests/test_payment_flow.py index aa3d698f49a..f382792081b 100644 --- a/addons/sale/tests/test_payment_flow.py +++ b/addons/sale/tests/test_payment_flow.py @@ -1,12 +1,15 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. + from unittest.mock import ANY, patch -from odoo.fields import Command +from odoo.exceptions import AccessError from odoo.tests import tagged, JsonRpcException from odoo.tools import mute_logger from odoo.addons.account_payment.tests.common import AccountPaymentCommon from odoo.addons.payment.tests.http_common import PaymentHttpCommon +from odoo.addons.portal.controllers.portal import CustomerPortal +from odoo.addons.sale.controllers.portal import PaymentPortal from odoo.addons.sale.tests.common import SaleCommon @@ -385,6 +388,50 @@ class TestSalePayment(AccountPaymentCommon, SaleCommon, PaymentHttpCommon): self.assertTrue(len(invoice) == 1) self.assertTrue(invoice.line_ids[0].is_downpayment) + def test_check_portal_access_token_before_rerouting_flow(self): + """ Test that access to the provided sales order is checked against the portal access token + before rerouting the payment flow. """ + payment_portal_controller = PaymentPortal() + + with patch.object(CustomerPortal, '_document_check_access') as mock: + payment_portal_controller._get_extra_payment_form_values() + self.assertEqual( + mock.call_count, 0, msg="No check should be made when sale_order_id is not provided." + ) + + mock.reset_mock() + + payment_portal_controller._get_extra_payment_form_values( + sale_order_id=self.sale_order.id, access_token='whatever' + ) + self.assertEqual( + mock.call_count, 1, msg="The check should be made as sale_order_id is provided." + ) + + def test_check_payment_access_token_before_rerouting_flow(self): + """ Test that access to the provided sales order is checked against the payment access token + before rerouting the payment flow. """ + payment_portal_controller = PaymentPortal() + + def _document_check_access_mock(*_args, **_kwargs): + raise AccessError('') + + with patch.object( + CustomerPortal, '_document_check_access', _document_check_access_mock + ), patch('odoo.addons.payment.utils.check_access_token') as check_payment_access_token_mock: + try: + payment_portal_controller._get_extra_payment_form_values( + sale_order_id=self.sale_order.id, access_token='whatever' + ) + except Exception: + pass # We don't care if it runs or not; we only count the calls. + self.assertEqual( + check_payment_access_token_mock.call_count, + 1, + msg="The access token should be checked again as a payment access token if the" + " check as a portal access token failed.", + ) + @mute_logger('odoo.http') def test_transaction_route_rejects_unexpected_kwarg(self): url = self._build_url(f'/my/orders/{self.sale_order.id}/transaction')