Build the BTCPay invoice server-side and validate the settled amount

Security: the checkout controller built the BTCPay invoice from the POSTed
redirect-form data (price, currency, orderId), and _extract_amount_data
returned None to opt out of amount validation, so the settled amount was
never checked against the transaction. A buyer could therefore submit an
arbitrary price and confirm an order after paying far less. The invoice is
now built server-side from the transaction, the redirect form only carries
the reference, and _extract_amount_data returns the amount and currency
settled on BTCPay (read back from the invoice in the IPN) so the framework
validates them against the transaction before it is set done.

Other fixes in the notification path:
- Fix the IPN notification URL: it was concatenated without a leading
  slash (base_url + "payment/btcpay/ipn"), producing an unreachable host
  so notifications never arrived. It is now built with urls.url_join.
- Handle the "expired" invoice status (cancel the transaction) instead of
  leaving it pending forever, and set an error on unknown statuses.

payment_btcpayserver/controllers/main.py

@@ -22,7 +22,6 @@
 import json
 import pprint
 
-import werkzeug
 from werkzeug import urls
 
 from odoo import _, http
@@ -41,85 +40,107 @@ class BTCPayController(http.Controller):
     _notify_url = '/payment/btcpay/ipn'
     _return_url = '/payment/btcpay/return'
 
-    @http.route(_checkout_url, type='http', auth='public', csrf=False, website=True)
-    def checkout(self, **data):
+    @staticmethod
+    def _btcpay_client(provider):
+        return BTCPayClient(
+            host=provider.btcpay_location,
+            pem=provider.btcpay_privateKey,
+            tokens={provider.btcpay_facade: provider.btcpay_token},
+        )
 
-        _logger.info("CHECKOUT: received data:\n%s", pprint.pformat(data))
-
-        # Look up the transaction by reference
-        reference = data.get('reference')
-        tx_sudo = request.env['payment.transaction'].sudo().search([
+    @staticmethod
+    def _find_transaction(reference):
+        return request.env['payment.transaction'].sudo().search([
             ('reference', '=', reference),
             ('provider_code', '=', 'btcpayserver'),
         ], limit=1)
+
+    @http.route(_checkout_url, type='http', auth='public', methods=['POST'],
+                csrf=False, website=True)
+    def checkout(self, **data):
+        """ Create the BTCPay invoice and redirect the buyer to it.
+
+        The redirect form only carries the transaction reference. The invoice
+        price, currency and buyer details are read from the transaction
+        server-side and are never taken from the (client-controlled) request
+        data, so a buyer cannot have an invoice created for a tampered amount.
+        """
+        _logger.info("BTCPay: checkout request with data:\n%s", pprint.pformat(data))
+        reference = data.get('reference')
+        tx_sudo = self._find_transaction(reference)
         if not tx_sudo:
             raise ValidationError(
-                _("BTCPay: No transaction found matching reference %s.", reference)
-            )
+                _("BTCPay: No transaction found matching reference %s.", reference))
+        if tx_sudo.state != 'draft':
+            # Already processed (e.g. double submission): fall back to the
+            # generic payment status page.
+            return request.redirect('/payment/status')
 
         provider = tx_sudo.provider_id
-        notification_url = str(data.get('notify_url')).replace("http://", "https://")
-        base_url = request.env['ir.config_parameter'].sudo().get_param('web.base.url')
-        redirect_url = urls.url_join(base_url, self._return_url)
-        client = BTCPayClient(host=provider.btcpay_location, pem=provider.btcpay_privateKey, tokens={provider.btcpay_facade: provider.btcpay_token})
-        invoice = client.create_invoice(
-            {"price": data.get('amount'),
-             "currency": data.get('currency_id'),
-             "orderId": data.get('reference'),
-             "token": provider.btcpay_token,
-             "redirectURL": redirect_url,
-             "notificationURL": notification_url,
-             "extendedNotifications": True,
-             "buyer": {"email": data.get('email') or 'noemailavailable@example.com',
-                       "name": data.get('name'),
-                       "address1": data.get('street'),
-                       "locality": data.get('city'),
-                       "postalCode": data.get('zip'),
-                       "country": data.get('country'),
-                       "notify": False}})
-        _logger.info('Invoice %s \n NOTIFY URL: %s', invoice, notification_url)
-        return werkzeug.utils.redirect(invoice['url'])
+        base_url = provider.get_base_url()
+        client = self._btcpay_client(provider)
+        invoice = client.create_invoice({
+            "price": tx_sudo.amount,
+            "currency": tx_sudo.currency_id.name,
+            "orderId": tx_sudo.reference,
+            "token": provider.btcpay_token,
+            "redirectURL": urls.url_join(base_url, self._return_url),
+            "notificationURL": urls.url_join(base_url, self._notify_url),
+            "extendedNotifications": True,
+            "buyer": {
+                "email": tx_sudo.partner_email or 'noemailavailable@example.com',
+                "name": tx_sudo.partner_name,
+                "address1": tx_sudo.partner_address,
+                "locality": tx_sudo.partner_city,
+                "postalCode": tx_sudo.partner_zip,
+                "country": tx_sudo.partner_country_id.code,
+                "notify": False,
+            },
+        })
+        tx_sudo.btcpay_invoiceId = invoice.get('id')
+        _logger.info("BTCPay: created invoice %s for transaction %s",
+                     invoice.get('id'), tx_sudo.reference)
+        return request.redirect(invoice['url'], local=False)
 
     @http.route(_notify_url, type='jsonrpc', auth='public', csrf=False)
     def btcpay_ipn(self, **post):
         """ BTCPay IPN. """
-        _logger.info('BTCPAY IPN RECEIVED...')
+        _logger.info('BTCPay: IPN received')
         data = json.loads(request.httprequest.data)
         _logger.info("%s", pprint.pformat(data))
         try:
             reference = data['data']['orderId']
             invoice_id = data['data']['id']
 
-            # Look up the transaction by reference
-            tx_sudo = request.env['payment.transaction'].sudo().search([
-                ('reference', '=', reference),
-                ('provider_code', '=', 'btcpayserver'),
-            ], limit=1)
+            tx_sudo = self._find_transaction(reference)
             if not tx_sudo:
                 _logger.warning("No transaction found matching reference %s.", reference)
                 return ''
 
-            provider = tx_sudo.provider_id
-            client = BTCPayClient(host=provider.btcpay_location, pem=provider.btcpay_privateKey,
-                                  tokens={provider.btcpay_facade: provider.btcpay_token})
-
+            # The invoice is fetched back from BTCPay (signed request); the
+            # posted payload is only used to locate the transaction.
+            client = self._btcpay_client(tx_sudo.provider_id)
             fetched_invoice = client.get_invoice(invoice_id)
-            _logger.info('fetched_invoice = %s', pprint.pformat(fetched_invoice))
+            _logger.info('BTCPay: fetched invoice = %s', pprint.pformat(fetched_invoice))
 
             payment_data = {
-                "reference": fetched_invoice['orderId'],
-                "status": fetched_invoice['status'],
-                "invoiceID": fetched_invoice['id'],
-                "txid": fetched_invoice['url'],
+                "reference": fetched_invoice.get('orderId'),
+                "status": fetched_invoice.get('status'),
+                "invoiceID": fetched_invoice.get('id'),
+                "txid": fetched_invoice.get('url'),
+                # Settled amount and currency, validated against the transaction
+                # by the payment framework before it is set done.
+                "amount": fetched_invoice.get('price'),
+                "currency": fetched_invoice.get('currency'),
             }
 
-            # Use the new Odoo 19 _process() pipeline
             tx_sudo._process('btcpayserver', payment_data)
         except ValidationError:
             _logger.exception("Unable to handle the notification data; skipping to acknowledge")
         return ''
 
-    @http.route(_return_url, type='http', auth="public", methods=['GET'], csrf=False, save_session=False)
+    @http.route(_return_url, type='http', auth="public", methods=['GET'],
+                csrf=False, save_session=False)
     def btcpay_return_from_redirect(self, **data):
         """ BTCPay return """
         _logger.info("BTCPay: user returned to shop after payment")

payment_btcpayserver/models/payment_transaction.py

@@ -2,7 +2,6 @@ import pprint
 
 from odoo import _, api, fields, models
 
-from odoo.addons.payment import utils as payment_utils
 from odoo.addons.payment.logging import get_payment_logger
 
 
@@ -15,13 +14,15 @@ class PaymentTransaction(models.Model):
     btcpay_invoiceId = fields.Char("Invoice Id")
     btcpay_txid = fields.Char("Transaction Id")
     btcpay_status = fields.Char("Transaction Status")
-    api_url = '/btcpay/checkout'
-    checkout_url = '/btcpay/checkout'
-    notify_url = 'payment/btcpay/ipn'
 
     def _get_specific_rendering_values(self, processing_values):
         """ Override of payment to return BTCPay-specific rendering values.
 
+        The redirect form only needs to carry the transaction reference: the
+        BTCPay invoice is built server-side (see the checkout controller) from
+        the transaction, so no amount, currency or buyer data is sent through
+        the browser.
+
         Note: self.ensure_one() from `_get_processing_values`
 
         :param dict processing_values: The generic and specific processing values of the transaction
@@ -29,29 +30,12 @@ class PaymentTransaction(models.Model):
         :rtype: dict
         """
         res = super()._get_specific_rendering_values(processing_values)
-
         if self.provider_code != 'btcpayserver':
             return res
 
-        base_url = self.provider_id.get_base_url()
-        partner_first_name, partner_last_name = payment_utils.split_partner_name(self.partner_name)
-
         return {
-            'address1': self.partner_address,
-            'amount': self.amount,
-            'city': self.partner_city,
-            'country': self.partner_country_id.code,
-            'currency_code': self.currency_id.name,
-            'email': self.partner_email,
-            'first_name': partner_first_name,
-            'item_name': f"{self.company_id.name}: {self.reference}",
-            'item_number': self.reference,
-            'last_name': partner_last_name,
-            'lc': self.partner_lang,
-            'state': self.partner_state_id.name,
-            'zip_code': self.partner_zip,
-            'api_url':  self.checkout_url,
-            'notify_url': base_url + self.notify_url,
+            'api_url': '/btcpay/checkout',
+            'reference': self.reference,
         }
 
     @api.model
@@ -69,19 +53,28 @@ class PaymentTransaction(models.Model):
         return payment_data.get('reference')
 
     def _extract_amount_data(self, payment_data):
-        """ Override of payment to skip amount validation for BTCPay.
+        """ Override of payment to return the settled amount and currency.
 
-        BTCPay invoices handle amount validation on the BTCPay server side,
-        so we skip the Odoo-side validation.
+        The amount and currency are read back from BTCPay on notification and
+        returned here so that the generic amount validation compares them
+        against the transaction. This prevents a tampered or mismatched invoice
+        from confirming the transaction (and thus the order).
 
         :param dict payment_data: The payment data sent by the provider.
-        :return: None to skip validation.
-        :rtype: None
+        :return: The settled amount data, or ``None`` for other providers.
+        :rtype: dict|None
         """
         if self.provider_code != 'btcpayserver':
             return super()._extract_amount_data(payment_data)
 
-        return None
+        try:
+            amount = float(payment_data.get('amount'))
+        except (TypeError, ValueError):
+            amount = None
+        return {
+            'amount': amount,
+            'currency_code': payment_data.get('currency'),
+        }
 
     def _apply_updates(self, payment_data):
         """ Override of payment to process the transaction based on BTCPay data.
@@ -106,13 +99,22 @@ class PaymentTransaction(models.Model):
             self._set_done()
         elif self.btcpay_status in ['new']:
             self.btcpay_invoiceId = payment_data.get('invoiceID')
-        elif self.btcpay_status in ['cancel', 'cancelled']:
-            self._set_canceled()
+        elif self.btcpay_status in ['expired', 'cancel', 'cancelled']:
+            self._set_canceled(
+                state_message="BTCPay: " + _("Invoice status: %s.", self.btcpay_status))
         elif self.btcpay_status in ['invalid']:
-            _logger.info(
+            _logger.warning(
                 "Received data with invalid payment status (%s) for transaction with reference %s",
                 self.btcpay_status, self.reference
             )
             self._set_error(
-                "BTCPay: " + _("Received data with invalid payment status: %s", self.btcpay_status)
+                "BTCPay: " + _("Received data with invalid payment status: %s.", self.btcpay_status)
+            )
+        else:
+            _logger.warning(
+                "Received data with unknown payment status (%s) for transaction with reference %s",
+                self.btcpay_status, self.reference
+            )
+            self._set_error(
+                "BTCPay: " + _("Received data with unknown payment status: %s.", self.btcpay_status)
             )

payment_btcpayserver/views/payment_btcpayserver_templates.xml

@@ -1,25 +1,14 @@
 <?xml version="1.0" encoding="utf-8"?>
 <odoo>
 
+    <!-- The redirect form is auto-submitted by the payment framework. It only
+         carries the transaction reference; the BTCPay invoice is built
+         server-side from the transaction (amount and currency cannot be
+         tampered with from the browser). -->
     <template id="redirect_form">
         <form t-att-action="api_url" method="post">
-            <input type="hidden" name="reference" t-att-value="item_number"/>
-            <input type="hidden" name="name" t-att-value="first_name"/>
-            <input type="hidden" name="email" t-att-value="email"/>
-            <input type="hidden" name="street" t-att-value="address1"/>
-            <input type="hidden" name="zip" t-att-value="zip_code"/>
-            <input type="hidden" name="city" t-att-value="city"/>
-            <input type="hidden" name="currency_id" t-att-value="currency_code"/>
-            <input type="hidden" name="country" t-att-value="country"/>
-            <input type="hidden" name="custom" t-att-value="item_name"/>
-            <input type="hidden" name="amount" t-att-value="amount"/>
-            <input type="hidden" name="notify_url" t-att-value="notify_url"/>
-            <button type="submit" width="100px" t-att-class="submit_class">
-                <img t-if="not submit_txt" src="/static/description/icon.png"/>
-                <span t-if="submit_txt"> <t t-esc="submit_txt"/> <span class="fa fa-long-arrow-right"/>
-                </span>
-            </button>
+            <input type="hidden" name="reference" t-att-value="reference"/>
         </form>
     </template>
 
-</odoo>
+</odoo>