diff --git a/payment_btcpayserver/controllers/main.py b/payment_btcpayserver/controllers/main.py index 6ca6d38..e2e4b13 100644 --- a/payment_btcpayserver/controllers/main.py +++ b/payment_btcpayserver/controllers/main.py @@ -20,7 +20,6 @@ # ****************************************************************************** import json -import pprint from werkzeug import urls @@ -55,6 +54,125 @@ class BTCPayController(http.Controller): ('provider_code', '=', 'btcpayserver'), ], limit=1) + @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): @@ -65,12 +183,13 @@ 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')) reference = data.get('reference') tx_sudo = self._find_transaction(reference) if not tx_sudo: raise ValidationError( _("BTCPay: No transaction found matching reference %s.", reference)) + self._lock_transaction(tx_sudo) if tx_sudo.state != 'draft': # Already processed (e.g. double submission): fall back to the # generic payment status page. @@ -79,6 +198,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, @@ -97,36 +241,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='jsonrpc', auth='public', csrf=False) + @http.route(_notify_url, type='jsonrpc', 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: - reference = data['data']['orderId'] - invoice_id = data['data']['id'] + data = json.loads(request.httprequest.data) + except (json.JSONDecodeError, TypeError, UnicodeDecodeError): + _logger.warning("BTCPay: received malformed JSON notification data") + return '' - tx_sudo = self._find_transaction(reference) - if not tx_sudo: - _logger.warning("No transaction found matching reference %s.", reference) - 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') - # The invoice is fetched back from BTCPay (signed request); the - # posted payload is only used to locate the transaction. + # 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(invoice_id) - _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, + ) payment_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, validated against the transaction # by the payment framework before it is set done. @@ -135,8 +297,8 @@ class BTCPayController(http.Controller): } tx_sudo._process('btcpayserver', payment_data) - except ValidationError: - _logger.exception("Unable to handle the notification data; skipping to acknowledge") + except ValidationError as error: + _logger.warning("BTCPay: rejected notification: %s", error) return '' @http.route(_return_url, type='http', auth="public", methods=['GET'], diff --git a/payment_btcpayserver/models/payment_transaction.py b/payment_btcpayserver/models/payment_transaction.py index 8008fdf..8ee7e7a 100644 --- a/payment_btcpayserver/models/payment_transaction.py +++ b/payment_btcpayserver/models/payment_transaction.py @@ -1,6 +1,7 @@ import pprint from odoo import _, api, fields, models +from odoo.exceptions import ValidationError from odoo.addons.payment.logging import get_payment_logger @@ -89,32 +90,43 @@ class PaymentTransaction(models.Model): _logger.info("BTCPay _apply_updates: %s", pprint.pformat(payment_data)) - self.provider_reference = payment_data.get('reference') - self.btcpay_txid = payment_data.get('txid') - self.btcpay_status = payment_data.get('status') + if not payment_data.get('reference'): + raise ValidationError( + "BTCPay: " + _("Received payment data with missing reference.")) - if self.btcpay_status in ['paid', 'processing']: + # Keep both external-reference fields bound to the authenticated + # BTCPay invoice. The controller validates this identity before + # entering `_process`. + if invoice_id := payment_data.get('invoiceID'): + self.btcpay_invoiceId = invoice_id + self.provider_reference = invoice_id + if payment_data.get('txid'): + self.btcpay_txid = payment_data['txid'] + status = payment_data.get('status') + self.btcpay_status = status + + if status in ('paid', 'processing'): self._set_pending(state_message=payment_data.get('pending_reason')) - elif self.btcpay_status in ['confirmed', 'complete']: + elif status in ('confirmed', 'complete'): self._set_done() - elif self.btcpay_status in ['new']: - self.btcpay_invoiceId = payment_data.get('invoiceID') - elif self.btcpay_status in ['expired', 'cancel', 'cancelled']: + 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.", self.btcpay_status)) - elif self.btcpay_status in ['invalid']: + state_message="BTCPay: " + _("Invoice status: %s.", status)) + elif status == 'invalid': _logger.warning( "Received data with invalid payment status (%s) for transaction with reference %s", - self.btcpay_status, self.reference + 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.", status) ) else: _logger.warning( "Received data with unknown payment status (%s) for transaction with reference %s", - self.btcpay_status, self.reference + status, self.reference ) self._set_error( - "BTCPay: " + _("Received data with unknown payment status: %s.", self.btcpay_status) + "BTCPay: " + _("Received data with unknown payment status: %s.", status) )