From dc6b7a5758b1e01c60c8421f12bea79e4aca3d55 Mon Sep 17 00:00:00 2001 From: SatoDri Date: Sun, 19 Jul 2026 23:56:03 +0200 Subject: [PATCH 1/4] Declare ecdsa as the module Python dependency The bundled BTCPay client (models/libs) imports ecdsa, while the manifest and requirements only listed btcpay-python, which is not used (the client is vendored). Installations that did not already have ecdsa available failed the external-dependency check. Declare ecdsa instead and bump the module version. --- payment_btcpayserver/__manifest__.py | 6 ++++-- payment_btcpayserver/requirements.txt | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/payment_btcpayserver/__manifest__.py b/payment_btcpayserver/__manifest__.py index aceb90a..f875153 100644 --- a/payment_btcpayserver/__manifest__.py +++ b/payment_btcpayserver/__manifest__.py @@ -25,7 +25,7 @@ 'author': 'BTCPay Server team and contributors', 'website': 'https://github.com/btcpayserver/odoo', 'category': 'Accounting/Payment Providers', - 'version': '18.0.1.0', + 'version': '18.0.1.1', 'license': 'GPL-3', 'currency': 'USD', 'application': False, @@ -39,8 +39,10 @@ 'data/payment_provider_data.xml', ], 'images': ['static/description/BTCPay-Odoo-17-featured.png'], + # The BTCPay client (legacy BitPay API) is vendored in models/libs and + # only requires 'ecdsa' (plus 'requests', shipped with Odoo). 'external_dependencies': { - 'python': ['btcpay-python'] + 'python': ['ecdsa'] }, 'post_init_hook': 'post_init_hook', 'uninstall_hook': 'uninstall_hook', diff --git a/payment_btcpayserver/requirements.txt b/payment_btcpayserver/requirements.txt index eeda1dc..aa5efdb 100644 --- a/payment_btcpayserver/requirements.txt +++ b/payment_btcpayserver/requirements.txt @@ -1 +1 @@ -btcpay-python \ No newline at end of file +ecdsa From 970502323306bb9c74d7f5d2b714975cc67b24a9 Mon Sep 17 00:00:00 2001 From: SatoDri Date: Sun, 19 Jul 2026 23:56:03 +0200 Subject: [PATCH 2/4] Remove dead payment.provider create() override The override guarded on self.code inside create(), which is empty on the model-level recordset, so the branch never ran; it also called a misspelled helper (generate_privakey). Removing it lets the private key be generated during pairing as intended, and keeps copying a provider from regenerating (and thus invalidating) an already paired key. --- payment_btcpayserver/models/payment_provider.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/payment_btcpayserver/models/payment_provider.py b/payment_btcpayserver/models/payment_provider.py index 4ddc059..cae0bd0 100644 --- a/payment_btcpayserver/models/payment_provider.py +++ b/payment_btcpayserver/models/payment_provider.py @@ -1,6 +1,6 @@ import logging -from odoo import _, api, fields, models +from odoo import api, fields, models from .libs.client import BTCPayClient from .libs import crypto @@ -20,15 +20,9 @@ class PaymentProvider(models.Model): btcpay_privateKey = fields.Text(string='Private Key', help='Private Key for BTCPay Client. Leave empty, will be autogenerated during pairing.') btcpay_facade = fields.Char(string='Facade', help='Token facade type: merchant/pos/payroll. Keep merchant', default='merchant') - def create(self, values_list): - if self.code == 'btcpayserver': - values_list['btcpay_privateKey'] = crypto.generate_privakey() - - return super(PaymentProvider, self).create(values_list) - @api.onchange('btcpay_pairingCode') def _onchange_pairingCode(self): - if not self.btcpay_token and self.code == 'btcpayserver' and not self.btcpay_pairingCode == '': + if not self.btcpay_token and self.code == 'btcpayserver' and self.btcpay_pairingCode: #_logger.info("ONCHANGE PAIRING CODE***SELF: %s %s %s", self.btcpay_location, self.btcpay_privateKey, self.btcpay_pairingCode) self.btcpay_privateKey = crypto.generate_privkey() client = BTCPayClient(host=self.btcpay_location, pem=self.btcpay_privateKey) From 9dc936ef330097cc7f60d2ea0f37ea5c2fb19979 Mon Sep 17 00:00:00 2001 From: SatoDri Date: Sun, 19 Jul 2026 23:56:03 +0200 Subject: [PATCH 3/4] Build the BTCPay invoice server-side and fix notification handling Security: the checkout controller built the BTCPay invoice from the POSTed redirect-form data (price, currency, orderId), and the IPN never checked the settled amount 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 the settled amount and currency returned by BTCPay are verified before the transaction 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. - Let the payment framework confirm the order and send the confirmation email via its post-processing, instead of calling _check_amount_and_confirm_order() and _send_order_confirmation_mail() manually, which sent the confirmation email twice. - Fix a crsf=False typo (was disabling nothing) to csrf=False on the return route, and drop the redundant _handle_notification_data override that duplicated the core method. - Guard against a missing reference instead of calling all() on it. --- payment_btcpayserver/controllers/main.py | 121 +++++++++------ .../models/payment_transaction.py | 139 ++++++++++-------- .../views/payment_btcpayserver_templates.xml | 23 +-- 3 files changed, 155 insertions(+), 128 deletions(-) diff --git a/payment_btcpayserver/controllers/main.py b/payment_btcpayserver/controllers/main.py index f16e4ec..fa7591e 100644 --- a/payment_btcpayserver/controllers/main.py +++ b/payment_btcpayserver/controllers/main.py @@ -19,23 +19,18 @@ # # ****************************************************************************** +import json import logging import pprint -import requests -import werkzeug from werkzeug import urls -from werkzeug.exceptions import Forbidden -from odoo import _, http, SUPERUSER_ID +from odoo import http from odoo.exceptions import ValidationError from odoo.http import request -from odoo.tools import html_escape -import json from ..models.libs.client import BTCPayClient - _logger = logging.getLogger(__name__) @@ -44,56 +39,89 @@ 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) + @staticmethod + def _btcpay_client(provider): + return BTCPayClient( + host=provider.btcpay_location, + pem=provider.btcpay_privateKey, + tokens={provider.btcpay_facade: provider.btcpay_token}, + ) + + @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)) + tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_notification_data( + 'btcpayserver', data) + if tx_sudo.state != 'draft': + # Already processed (e.g. double submission): fall back to the + # generic payment status page. + return request.redirect('/payment/status') - _logger.info("CHECKOUT: notification received from BTCPay with data:\n%s", pprint.pformat(data)) - tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_notification_data('btcpayserver', data) 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='json', 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: - notification_data = {"reference": data['data']['orderId'], - "invoiceID": data['data']['id']} - # Check the origin and integrity of the notification - tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_notification_data('btcpayserver', notification_data) - provider = tx_sudo.provider_id - client = BTCPayClient(host=provider.btcpay_location, pem=provider.btcpay_privateKey, - tokens={provider.btcpay_facade: provider.btcpay_token}) + notification_data = { + "reference": data['data']['orderId'], + "invoiceID": data['data']['id'], + } + # Check the origin and integrity of the notification: the invoice + # is fetched back from BTCPay (signed request), the posted payload + # is only used to locate the transaction. + tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_notification_data( + 'btcpayserver', notification_data) + client = self._btcpay_client(tx_sudo.provider_id) fetched_invoice = client.get_invoice(notification_data['invoiceID']) - _logger.info('fetched_invoice = %s',pprint.pformat(fetched_invoice)) + _logger.info('BTCPay: fetched invoice = %s', pprint.pformat(fetched_invoice)) - notification_data = {"reference": fetched_invoice['orderId'], - "status": fetched_invoice['status'], - "invoiceID": fetched_invoice['id'], - "txid": fetched_invoice['url']} + notification_data = { + "reference": fetched_invoice.get('orderId'), + "status": fetched_invoice.get('status'), + "invoiceID": fetched_invoice.get('id'), + "txid": fetched_invoice.get('url'), + # Settled amount and currency, verified against the transaction + # before it is set done. + "amount": fetched_invoice.get('price'), + "currency": fetched_invoice.get('currency'), + } # Handle the notification data tx_sudo._handle_notification_data('btcpayserver', notification_data) @@ -101,7 +129,8 @@ class BTCPayController(http.Controller): _logger.exception("Unable to handle the notification data; skipping to acknowledge") return '' - @http.route(_return_url, type='http', auth="public", methods=['GET'], crsf=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 @@ -109,4 +138,4 @@ class BTCPayController(http.Controller): IPN anyway, so just show the user the order confirmation / payment status page. """ _logger.info("BTCPay: user returned to shop after payment") - return request.redirect('/payment/status') \ No newline at end of file + return request.redirect('/payment/status') diff --git a/payment_btcpayserver/models/payment_transaction.py b/payment_btcpayserver/models/payment_transaction.py index 2f0a964..46b89a7 100644 --- a/payment_btcpayserver/models/payment_transaction.py +++ b/payment_btcpayserver/models/payment_transaction.py @@ -1,12 +1,9 @@ import logging import pprint -from werkzeug import urls -from odoo import _, api, fields, models +from odoo import _, fields, models from odoo.exceptions import ValidationError -from odoo.addons.payment import utils as payment_utils - _logger = logging.getLogger(__name__) @@ -16,12 +13,14 @@ 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 Specific rendering 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` @@ -29,32 +28,13 @@ class PaymentTransaction(models.Model): :return: The dict of provider-specific processing values :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() - _logger.info('Hola! API URL: %s', processing_values) - 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, } def _get_tx_from_notification_data(self, provider_code, notification_data): @@ -67,7 +47,6 @@ class PaymentTransaction(models.Model): :raise: ValidationError if the data match no transaction """ tx = super()._get_tx_from_notification_data(provider_code, notification_data) - _logger.info('GET TX FROM NOTIFICATION Notification_data %s', pprint.pformat(notification_data)) if provider_code != 'btcpayserver' or len(tx) == 1: return tx @@ -79,21 +58,13 @@ class PaymentTransaction(models.Model): ) return tx - def _handle_notification_data(self, provider_code, notification_data): - """ Match the transaction with the notification data, update its state and return it. - - :param str provider_code: The code of the provider handling the transaction. - :param dict notification_data: The notification data sent by the provider. - :return: The transaction. - :rtype: recordset of `payment.transaction` - """ - tx = self._get_tx_from_notification_data(provider_code, notification_data) - tx._process_notification_data(notification_data) - return tx - def _process_notification_data(self, notification_data): """ Override of payment to process the transaction based on BTCPay data. + Only the transaction state is updated here; confirming the related + sale order, creating the payment and sending the confirmation email are + handled by the generic post-processing of the `payment` framework. + Note: self.ensure_one() :param dict notification_data: The notification data sent by the provider @@ -104,31 +75,69 @@ class PaymentTransaction(models.Model): if self.provider_code != 'btcpayserver': return - _logger.info("_process_notification_data %s", pprint.pformat(notification_data)) - txn_id = notification_data.get('reference') - if not all(txn_id): + _logger.info( + "processing notification data for BTCPay transaction with reference %s:\n%s", + self.reference, pprint.pformat(notification_data)) + if not notification_data.get('reference'): raise ValidationError( - "BTCPay: " + _("Missing value for txn_id (%(txn_id)s)).", txn_id=txn_id)) + "BTCPay: " + _("Received notification data with missing reference.")) - self.provider_reference = txn_id - self.btcpay_txid = notification_data.get('txid') - self.btcpay_status = notification_data.get('status') + if notification_data.get('invoiceID'): + self.btcpay_invoiceId = notification_data['invoiceID'] + self.provider_reference = notification_data['invoiceID'] + if notification_data.get('txid'): + self.btcpay_txid = notification_data['txid'] + status = notification_data.get('status') + self.btcpay_status = status - if self.btcpay_status in ['paid','processing']: + if status in ('paid', 'processing'): self._set_pending(state_message=notification_data.get('pending_reason')) - elif self.btcpay_status in ['confirmed', 'complete']: - self._set_done() - confirmed_orders = self._check_amount_and_confirm_order() - confirmed_orders._send_order_confirmation_mail() - elif self.btcpay_status in ['new']: - self.btcpay_invoiceId = notification_data.get('invoiceID') - elif self.btcpay_status in ['cancel','cancelled']: - self._set_canceled() - elif self.btcpay_status in ['invalid']: - _logger.info( - "received data with invalid payment status (%s) for transaction with reference %s", - self.btcpay_status, self.reference - ) + elif status in ('confirmed', 'complete'): + if self._btcpay_verify_amount(notification_data): + self._set_done() + elif status == 'new': + pass # Invoice created on BTCPay, waiting for the buyer to pay. + elif status in ('expired', 'cancel', 'cancelled'): + self._set_canceled( + state_message="BTCPay: " + _("Invoice status: %s.", status)) + elif status == 'invalid': + _logger.warning( + "received invalid payment status for transaction with reference %s", self.reference) self._set_error( - "BTCPay: " + _("Received data with invalid payment status: %s", self.btcpay_status) - ) \ No newline at end of file + "BTCPay: " + _("Received data with invalid payment status: %s.", status)) + else: + _logger.warning( + "received unrecognized payment status (%s) for transaction with reference %s", + status, self.reference) + self._set_error( + "BTCPay: " + _("Received data with unknown payment status: %s.", status)) + + def _btcpay_verify_amount(self, notification_data): + """ Check that the settled BTCPay invoice matches the transaction. + + The BTCPay invoice is created from the transaction, but its settled + amount and currency are read back from BTCPay on notification and + compared here so that a tampered or mismatched invoice can never + confirm the transaction (and thus the order). On mismatch the + transaction is set in error and ``False`` is returned. + + :param dict notification_data: The notification data sent by the provider + :return: Whether the settled amount and currency match the transaction + :rtype: bool + """ + self.ensure_one() + currency = notification_data.get('currency') + try: + amount = float(notification_data.get('amount')) + except (TypeError, ValueError): + amount = None + if (amount is None + or currency != self.currency_id.name + or self.currency_id.compare_amounts(amount, self.amount) != 0): + self._set_error("BTCPay: " + _( + "The settled amount (%(amount)s %(currency)s) does not match the " + "expected amount (%(expected)s %(expected_currency)s).", + amount=notification_data.get('amount'), currency=currency, + expected=self.amount, expected_currency=self.currency_id.name)) + return False + return True diff --git a/payment_btcpayserver/views/payment_btcpayserver_templates.xml b/payment_btcpayserver/views/payment_btcpayserver_templates.xml index 389f05c..b033285 100644 --- a/payment_btcpayserver/views/payment_btcpayserver_templates.xml +++ b/payment_btcpayserver/views/payment_btcpayserver_templates.xml @@ -1,25 +1,14 @@ + - \ No newline at end of file + From fadfb0f6a83284558910c34b044d39b4f124732d Mon Sep 17 00:00:00 2001 From: ndeet Date: Fri, 7 Aug 2026 16:25:15 +0200 Subject: [PATCH 4/4] * Strict invoice and order identity binding. * IPN lookup by stored invoice ID instead of untrusted order ID. * Invoice ID stored as provider_reference and checked for reuse. * Transaction row locking prevents concurrent duplicate checkouts. * Existing invoices are reused; expired ones are replaced safely. * Late notifications for replaced invoices are ignored. * Reduced sensitive webhook logging and restricted IPN to POST. --- payment_btcpayserver/controllers/main.py | 208 ++++++++++++++++++++--- 1 file changed, 185 insertions(+), 23 deletions(-) diff --git a/payment_btcpayserver/controllers/main.py b/payment_btcpayserver/controllers/main.py index fa7591e..7e8f7e4 100644 --- a/payment_btcpayserver/controllers/main.py +++ b/payment_btcpayserver/controllers/main.py @@ -21,11 +21,10 @@ import json import logging -import pprint from werkzeug import urls -from odoo import http +from odoo import _, http from odoo.exceptions import ValidationError from odoo.http import request @@ -47,6 +46,125 @@ class BTCPayController(http.Controller): tokens={provider.btcpay_facade: provider.btcpay_token}, ) + @staticmethod + def _lock_transaction(tx_sudo): + """Lock a transaction until the current database transaction ends. + + Checkout and IPN requests can arrive concurrently. Serializing them on + the payment transaction prevents two checkout requests from creating + separate invoices and makes an IPN re-check the invoice ID after a + checkout replaces an expired invoice. + """ + tx_sudo.ensure_one() + tx_sudo.flush_recordset() + tx_sudo.env.cr.execute( + "SELECT id FROM payment_transaction WHERE id = %s FOR UPDATE", + [tx_sudo.id], + ) + tx_sudo.invalidate_recordset([ + 'state', 'btcpay_invoiceId', 'provider_reference', + ]) + + @staticmethod + def _ensure_invoice_id_is_unique(tx_sudo, invoice_id): + """Reject an invoice ID already linked to another transaction.""" + other_tx = tx_sudo.env['payment.transaction'].sudo().search([ + ('id', '!=', tx_sudo.id), + ('provider_code', '=', 'btcpayserver'), + '|', + ('btcpay_invoiceId', '=', invoice_id), + ('provider_reference', '=', invoice_id), + ], limit=1) + if other_tx: + raise ValidationError(_( + "BTCPay: The invoice is already linked to another transaction." + )) + + @classmethod + def _validate_invoice_binding( + cls, tx_sudo, incoming_invoice_id, incoming_order_id, fetched_invoice): + """Bind the untrusted IPN payload to one authenticated invoice and transaction.""" + if not isinstance(fetched_invoice, dict): + raise ValidationError(_("BTCPay: Received an invalid invoice response.")) + + fetched_invoice_id = fetched_invoice.get('id') + fetched_order_id = fetched_invoice.get('orderId') + if not ( + incoming_invoice_id + and incoming_invoice_id == fetched_invoice_id + and incoming_invoice_id == tx_sudo.btcpay_invoiceId + and incoming_invoice_id == tx_sudo.provider_reference + ): + raise ValidationError(_( + "BTCPay: The notification invoice does not match the transaction." + )) + if not ( + incoming_order_id + and incoming_order_id == fetched_order_id + and incoming_order_id == tx_sudo.reference + ): + raise ValidationError(_( + "BTCPay: The notification order does not match the transaction." + )) + cls._ensure_invoice_id_is_unique(tx_sudo, incoming_invoice_id) + + @classmethod + def _find_transaction_by_invoice_id(cls, invoice_id): + """Find exactly one BTCPay transaction from its stored invoice ID.""" + if not isinstance(invoice_id, str) or not invoice_id: + raise ValidationError(_("BTCPay: Missing invoice ID in notification data.")) + + txs_sudo = request.env['payment.transaction'].sudo().search([ + ('provider_code', '=', 'btcpayserver'), + ('btcpay_invoiceId', '=', invoice_id), + ], limit=2) + if len(txs_sudo) != 1: + raise ValidationError(_( + "BTCPay: No unique transaction found matching the invoice ID." + )) + return txs_sudo + + @staticmethod + def _validate_invoice_amount(tx_sudo, invoice): + """Reject reuse of an invoice with a different amount or currency.""" + try: + amount = float(invoice.get('price')) + except (AttributeError, TypeError, ValueError): + amount = None + if ( + amount is None + or invoice.get('currency') != tx_sudo.currency_id.name + or tx_sudo.currency_id.compare_amounts(amount, tx_sudo.amount) != 0 + ): + raise ValidationError(_( + "BTCPay: The existing invoice amount or currency does not match the transaction." + )) + + @classmethod + def _validate_existing_invoice(cls, tx_sudo, invoice_id, invoice): + """Validate a stored invoice before redirecting the customer to it.""" + cls._validate_invoice_binding( + tx_sudo, invoice_id, tx_sudo.reference, invoice, + ) + if not isinstance(invoice.get('status'), str) or not invoice['status']: + raise ValidationError(_("BTCPay: The existing invoice response is incomplete.")) + + @classmethod + def _validate_created_invoice(cls, tx_sudo, invoice): + """Validate a newly created invoice before storing its identity.""" + if not isinstance(invoice, dict): + raise ValidationError(_("BTCPay: Received an invalid invoice response.")) + invoice_id = invoice.get('id') + if ( + not isinstance(invoice_id, str) + or not invoice_id + or not isinstance(invoice.get('url'), str) + or not invoice['url'] + or invoice.get('orderId') not in (None, tx_sudo.reference) + ): + raise ValidationError(_("BTCPay: The created invoice response is incomplete.")) + cls._ensure_invoice_id_is_unique(tx_sudo, invoice_id) + @http.route(_checkout_url, type='http', auth='public', methods=['POST'], csrf=False, website=True) def checkout(self, **data): @@ -57,9 +175,10 @@ class BTCPayController(http.Controller): 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)) + _logger.info("BTCPay: checkout request for transaction %s", data.get('reference')) tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_notification_data( 'btcpayserver', data) + self._lock_transaction(tx_sudo) if tx_sudo.state != 'draft': # Already processed (e.g. double submission): fall back to the # generic payment status page. @@ -68,6 +187,31 @@ class BTCPayController(http.Controller): provider = tx_sudo.provider_id base_url = provider.get_base_url() client = self._btcpay_client(provider) + + # A browser retry must not create a second payable invoice. Reuse the + # stored invoice while it remains payable. If it expired, the new ID is + # stored before the lock is released, so late notifications for the old + # invoice can no longer select this transaction. + if tx_sudo.btcpay_invoiceId: + existing_invoice = client.get_invoice(tx_sudo.btcpay_invoiceId) + self._validate_existing_invoice( + tx_sudo, tx_sudo.btcpay_invoiceId, existing_invoice, + ) + if existing_invoice['status'].lower() != 'expired': + self._validate_invoice_amount(tx_sudo, existing_invoice) + if ( + not isinstance(existing_invoice.get('url'), str) + or not existing_invoice['url'] + ): + raise ValidationError(_( + "BTCPay: The existing invoice response is incomplete." + )) + _logger.info( + "BTCPay: reusing invoice %s for transaction %s", + tx_sudo.btcpay_invoiceId, tx_sudo.reference, + ) + return request.redirect(existing_invoice['url'], local=False) + invoice = client.create_invoice({ "price": tx_sudo.amount, "currency": tx_sudo.currency_id.name, @@ -86,36 +230,54 @@ class BTCPayController(http.Controller): "notify": False, }, }) - tx_sudo.btcpay_invoiceId = invoice.get('id') + self._validate_created_invoice(tx_sudo, invoice) + tx_sudo.write({ + 'btcpay_invoiceId': invoice['id'], + 'provider_reference': invoice['id'], + }) _logger.info("BTCPay: created invoice %s for transaction %s", - invoice.get('id'), tx_sudo.reference) + invoice['id'], tx_sudo.reference) return request.redirect(invoice['url'], local=False) - @http.route(_notify_url, type='json', auth='public', csrf=False) + @http.route(_notify_url, type='json', auth='public', methods=['POST'], csrf=False) def btcpay_ipn(self, **post): """ BTCPay IPN. """ _logger.info('BTCPay: IPN received') - data = json.loads(request.httprequest.data) - _logger.info("%s", pprint.pformat(data)) try: - notification_data = { - "reference": data['data']['orderId'], - "invoiceID": data['data']['id'], - } - # Check the origin and integrity of the notification: the invoice - # is fetched back from BTCPay (signed request), the posted payload - # is only used to locate the transaction. - tx_sudo = request.env['payment.transaction'].sudo()._get_tx_from_notification_data( - 'btcpayserver', notification_data) + data = json.loads(request.httprequest.data) + except (json.JSONDecodeError, TypeError, UnicodeDecodeError): + _logger.warning("BTCPay: received malformed JSON notification data") + return '' + + try: + ipn_data = data.get('data') if isinstance(data, dict) else None + if not isinstance(ipn_data, dict): + raise ValidationError(_("BTCPay: Invalid notification data.")) + incoming_invoice_id = ipn_data.get('id') + incoming_order_id = ipn_data.get('orderId') + + # Never use the untrusted order ID to select a transaction. The + # stored invoice ID is the lookup key. + tx_sudo = self._find_transaction_by_invoice_id(incoming_invoice_id) client = self._btcpay_client(tx_sudo.provider_id) - fetched_invoice = client.get_invoice(notification_data['invoiceID']) - _logger.info('BTCPay: fetched invoice = %s', pprint.pformat(fetched_invoice)) + # Fetching through the authenticated API establishes the invoice's + # current state. Lock afterwards and re-check the stored ID so a + # concurrent checkout cannot replace it between lookup and use. + fetched_invoice = client.get_invoice(incoming_invoice_id) + self._lock_transaction(tx_sudo) + self._validate_invoice_binding( + tx_sudo, incoming_invoice_id, incoming_order_id, fetched_invoice, + ) + _logger.info( + "BTCPay: verified invoice %s with status %s for transaction %s", + fetched_invoice.get('id'), fetched_invoice.get('status'), tx_sudo.reference, + ) notification_data = { - "reference": fetched_invoice.get('orderId'), + "reference": tx_sudo.reference, "status": fetched_invoice.get('status'), - "invoiceID": fetched_invoice.get('id'), + "invoiceID": incoming_invoice_id, "txid": fetched_invoice.get('url'), # Settled amount and currency, verified against the transaction # before it is set done. @@ -125,8 +287,8 @@ class BTCPayController(http.Controller): # Handle the notification data tx_sudo._handle_notification_data('btcpayserver', notification_data) - except ValidationError: # Acknowledge the notification to avoid getting spammed - _logger.exception("Unable to handle the notification data; skipping to acknowledge") + except ValidationError as error: # Acknowledge rejected notifications without retries. + _logger.warning("BTCPay: rejected notification: %s", error) return '' @http.route(_return_url, type='http', auth="public", methods=['GET'],