[FIX] (account_)payment, sale: check document access in payment routes

Commit 4d1a1f1c introduced a systematic check on the kwargs passed to
transaction routes of modules integrating with online payments, but
failed to check the access token of documents whose ID is passed to
payment routes. This allowed retrieving the access token of such
documents by visiting a route that did not check the document access
(e.g., /my/payment_method) and passing an arbitrary document ID
(e.g, sale_order_id=123). The route's controller would reroute the
payment flow to the document's portal page and render the landing route
of the flow on the payment form, with the access token included.

This commit makes sure that we always check the access token of a
document before reading rerouting a payment flow.

closes odoo/odoo#138238

Signed-off-by: Antoine Vandevenne (anv) <anv@odoo.com>

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':

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}/')

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)
 

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':

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')