diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 9221189..5588e72 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -22,3 +22,32 @@ jobs: - name: Validate module run: python scripts/validate.py + + odoo18: + runs-on: ubuntu-latest + services: + postgres: + image: postgres:16-alpine + env: + POSTGRES_USER: odoo + POSTGRES_PASSWORD: odoo-test + POSTGRES_DB: postgres + ports: + - 5432:5432 + options: >- + --health-cmd "pg_isready -U odoo" + --health-interval 5s + --health-timeout 5s + --health-retries 10 + steps: + - name: Checkout + uses: actions/checkout@v5 + + - name: Install and test in Odoo 18 + run: | + docker run --rm --network host --entrypoint odoo \ + -v "$GITHUB_WORKSPACE:/mnt/extra-addons:ro" \ + odoo:18.0 \ + --db_host=127.0.0.1 --db_user=odoo --db_password=odoo-test \ + -d makepay_test -i makepay_odoo --test-enable --test-tags /makepay_odoo \ + --stop-after-init --without-demo=all --http-interface=127.0.0.1 --log-level=test diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..4c491fc --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,11 @@ +# Changelog + +## 18.0.1.0.1 — 2026-10-13 + +- Fix unauthenticated payment-state changes through the browser return route. Return parameters no longer reconcile transactions; payment confirmation requires a signed webhook. +- Accept MakePay's plain JSON webhook deliveries and recognize completed payments. +- Reject malformed webhook payloads and match paid event names exactly. +- Add Odoo 18 HTTP regression tests for return forgery, webhook authentication, reconciliation, and checkout rendering. +- Clarify that this release supports Odoo 18. Odoo 19 requires different payment notification hooks. + +Thanks to **kta1kri** for privately reporting the return-route vulnerability. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 845d4dc..0c97df9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,4 +4,12 @@ Thanks for helping improve the MakePay Odoo payment provider. Run `python scripts/validate.py` before opening a pull request. Test checkout redirection, return handling, and webhook reconciliation in an Odoo development database before release. +Run the module's HTTP regression suite with: + +```sh +odoo -d makepay_test -i makepay_odoo --test-enable --test-tags /makepay_odoo --stop-after-init --without-demo=all +``` + +Add this repository to Odoo's addons path and use a disposable development database. GitHub Actions runs this suite in Odoo 18 with PostgreSQL 16. + Do not commit MakePay keys, Odoo database dumps, production transaction payloads, webhook signing secrets, or customer data. diff --git a/README.md b/README.md index 5d650dd..f4b066d 100644 --- a/README.md +++ b/README.md @@ -6,8 +6,8 @@ The module adds a MakePay provider, creates hosted payment links for Odoo transa ## Target Versions -- Odoo 18 and 19 payment provider APIs -- Odoo 17 may work with minor view or method adjustments +- Odoo 18 payment provider API +- Odoo 19 is not supported by this release; its payment notification hooks differ. ## Configuration @@ -34,4 +34,6 @@ The validator compiles Python files, checks XML data files, verifies required mo ## Production Notes -Run this module in an Odoo development database before production use. The provider hooks follow Odoo's standard payment provider extension points, but each Odoo major version can make small view or provider-field changes. +Upgrade installed modules to **18.0.1.0.1**. Browser return URLs only redirect to Odoo's payment status page; signed MakePay webhooks are required to confirm payments. Configure the webhook URL and secret on both sides before accepting payments. A customer may see a pending status until the webhook arrives. + +Run this module in an Odoo development database before production use. The provider hooks follow Odoo 18's payment provider extension points. diff --git a/makepay_odoo/__manifest__.py b/makepay_odoo/__manifest__.py index 1b2cd3b..db4b9dc 100644 --- a/makepay_odoo/__manifest__.py +++ b/makepay_odoo/__manifest__.py @@ -1,6 +1,6 @@ { "name": "MakePay Payment Provider", - "version": "18.0.1.0.0", + "version": "18.0.1.0.1", "category": "Accounting/Payment Providers", "summary": "Accept MakePay hosted payment links in Odoo.", "author": "MakePay", diff --git a/makepay_odoo/controllers/main.py b/makepay_odoo/controllers/main.py index c3abc18..62849ca 100644 --- a/makepay_odoo/controllers/main.py +++ b/makepay_odoo/controllers/main.py @@ -6,6 +6,7 @@ from hashlib import sha256 from odoo import http +from odoo.exceptions import ValidationError from odoo.http import request _logger = logging.getLogger(__name__) @@ -17,30 +18,42 @@ class MakePayController(http.Controller): @http.route(_return_url, type="http", auth="public", methods=["GET"], csrf=False, save_session=False) def makepay_return(self, **data): - request.env["payment.transaction"].sudo()._handle_notification_data("makepay", data) + # Browser return parameters are untrusted. Only signed webhooks reconcile payments. return request.redirect("/payment/status") - @http.route(_webhook_url, type="json", auth="public", methods=["POST"], csrf=False, save_session=False) - def makepay_webhook(self): - raw_body = request.httprequest.get_data() or b"{}" + @http.route(_webhook_url, type="http", auth="public", methods=["POST"], csrf=False, save_session=False) + def makepay_webhook(self, **kwargs): + raw_body = request.httprequest.get_data() signature = request.httprequest.headers.get("X-MakePay-Signature") - payload = json.loads(raw_body.decode("utf-8")) - tx_sudo = request.env["payment.transaction"].sudo()._get_tx_from_notification_data( - "makepay", - payload, - ) + try: + payload = json.loads(raw_body.decode("utf-8")) + except (UnicodeDecodeError, ValueError): + return request.make_json_response({"error": "invalid payload"}, status=400) + + if not isinstance(payload, dict) or any( + key in payload and not isinstance(payload[key], dict) + for key in ("paymentLink", "data", "event", "session") + ): + return request.make_json_response({"error": "invalid payload"}, status=400) + + try: + tx_sudo = request.env["payment.transaction"].sudo()._get_tx_from_notification_data( + "makepay", payload, + ) + except ValidationError: + return request.make_json_response({"error": "missing payment link UID"}, status=400) if not tx_sudo: _logger.info("MakePay webhook ignored because no matching transaction was found.") - return {"ok": True, "reconciled": False} + return request.make_json_response({"ok": True, "reconciled": False}) provider = tx_sudo.provider_id if not self._verify_signature(raw_body, signature, provider.makepay_webhook_secret): _logger.warning("MakePay webhook rejected because signature verification failed.") return request.make_json_response({"error": "invalid signature"}, status=401) - tx_sudo._handle_notification_data("makepay", payload) - return {"ok": True, "reconciled": True} + tx_sudo._process_notification_data(payload) + return request.make_json_response({"ok": True, "reconciled": True}) @staticmethod def _verify_signature(raw_body: bytes, signature_header: str | None, secret: str | None) -> bool: diff --git a/makepay_odoo/models/payment_transaction.py b/makepay_odoo/models/payment_transaction.py index 04c70be..c4d855f 100644 --- a/makepay_odoo/models/payment_transaction.py +++ b/makepay_odoo/models/payment_transaction.py @@ -26,8 +26,8 @@ def _get_specific_rendering_values(self, processing_values): "orderId": self.reference, "customerEmail": self.partner_email, "returnUrl": f"{base_url}{MakePayController._return_url}?reference={self.reference}", - "successUrl": f"{base_url}{MakePayController._return_url}?reference={self.reference}&status=paid", - "failureUrl": f"{base_url}{MakePayController._return_url}?reference={self.reference}&status=failed", + "successUrl": f"{base_url}{MakePayController._return_url}", + "failureUrl": f"{base_url}{MakePayController._return_url}", "metadata": { "platform": "odoo", "reference": self.reference, @@ -110,7 +110,10 @@ def _makepay_payment_link_uid(notification_data): @staticmethod def _makepay_is_paid(notification_data): values = PaymentTransaction._makepay_status_values(notification_data) - return any(value == "paid" or "payment.paid" in value for value in values) + return bool(values & { + "paid", "complete", "payment.paid", "makepay.payment.paid", + "payment.complete", "makepay.payment.complete", + }) @staticmethod def _makepay_is_failed(notification_data): diff --git a/makepay_odoo/tests/__init__.py b/makepay_odoo/tests/__init__.py new file mode 100644 index 0000000..62c6a43 --- /dev/null +++ b/makepay_odoo/tests/__init__.py @@ -0,0 +1 @@ +from . import test_payment diff --git a/makepay_odoo/tests/test_payment.py b/makepay_odoo/tests/test_payment.py new file mode 100644 index 0000000..c5828a8 --- /dev/null +++ b/makepay_odoo/tests/test_payment.py @@ -0,0 +1,162 @@ +import hmac +import json +import time +from hashlib import sha256 +from unittest.mock import patch + +from odoo.tests import tagged +from odoo.addons.payment.tests.http_common import PaymentHttpCommon + +from ..controllers.main import MakePayController + + +@tagged("post_install", "-at_install") +class TestMakePayPayment(PaymentHttpCommon): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.provider = cls._prepare_provider("makepay", update_values={ + "makepay_key_id": "test-key-id", + "makepay_key_secret": "test-key-secret", + "makepay_webhook_secret": "test-webhook-secret", + }) + cls.payment_method = cls.env.ref("makepay_odoo.payment_method_makepay") + cls.payment_method_id = cls.payment_method.id + + def setUp(self): + super().setUp() + self.tx = self._create_transaction( + "redirect", state="pending", makepay_payment_link_uid="test-link-uid", + provider_reference="test-link-uid", + ) + + def _payload(self, status="complete"): + return { + "type": "makepay.payment.status_changed", + "event": {"type": "status_changed"}, + "paymentLink": {"uid": "test-link-uid", "status": "active"}, + "session": {"status": status}, + "data": {}, + } + + def _post_webhook(self, payload, secret="test-webhook-secret", signature=None): + body = json.dumps(payload).encode() + if signature is None: + timestamp = str(int(time.time())) + digest = hmac.new(secret.encode(), timestamp.encode() + b"." + body, sha256).hexdigest() + signature = f"t={timestamp},v1={digest}" + return self.opener.post( + self._build_url(MakePayController._webhook_url), data=body, + headers={"Content-Type": "application/json", "X-MakePay-Signature": signature}, + ) + + def test_browser_returns_cannot_change_payment_state(self): + for parameters in ( + {"reference": "test-link-uid", "status": "paid"}, + {"reference": "test-link-uid", "status": "PAID"}, + {"reference": "test-link-uid", "type": "makepay.payment.paid"}, + {"reference": "test-link-uid", "status": "complete"}, + {"reference": "test-link-uid", "status": "failed"}, + {"reference": "test-link-uid", "status": "canceled"}, + {"reference": self.tx.reference, "status": "paid"}, + {}, + ): + with self.subTest(parameters=parameters): + response = self.opener.get( + self._build_url(MakePayController._return_url), + params=parameters, allow_redirects=False, + ) + self.assertEqual(response.status_code, 303) + self.assertEqual(response.headers["Location"], "/payment/status") + self.tx.invalidate_recordset() + self.assertEqual(self.tx.state, "pending") + self.assertEqual(self.tx.provider_reference, "test-link-uid") + + def test_valid_plain_json_webhook_confirms_payment(self): + response = self._post_webhook(self._payload()) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), {"ok": True, "reconciled": True}) + self.tx.invalidate_recordset() + self.assertEqual(self.tx.state, "done") + # Retries and subsequent browser returns preserve a confirmed transaction. + self.assertEqual(self._post_webhook(self._payload()).status_code, 200) + self.opener.get(self._build_url(MakePayController._return_url), params={ + "reference": "test-link-uid", "status": "failed", + }, allow_redirects=False) + self.tx.invalidate_recordset() + self.assertEqual(self.tx.state, "done") + + def test_unsigned_and_wrong_secret_webhooks_do_not_confirm_payment(self): + for kwargs in ({"signature": ""}, {"secret": "wrong-secret"}): + with self.subTest(kwargs=kwargs): + response = self._post_webhook(self._payload(), **kwargs) + self.assertEqual(response.status_code, 401) + self.tx.invalidate_recordset() + self.assertEqual(self.tx.state, "pending") + + def test_tampered_signed_body_does_not_confirm_payment(self): + body = json.dumps(self._payload("pending")).encode() + timestamp = str(int(time.time())) + digest = hmac.new(b"test-webhook-secret", timestamp.encode() + b"." + body, sha256).hexdigest() + response = self._post_webhook(self._payload(), signature=f"t={timestamp},v1={digest}") + self.assertEqual(response.status_code, 401) + self.tx.invalidate_recordset() + self.assertEqual(self.tx.state, "pending") + + def test_missing_webhook_secret_fails_closed(self): + self.provider.makepay_webhook_secret = False + response = self._post_webhook(self._payload()) + self.assertEqual(response.status_code, 401) + self.tx.invalidate_recordset() + self.assertEqual(self.tx.state, "pending") + + def test_unknown_link_does_not_modify_existing_transaction(self): + payload = self._payload() + payload["paymentLink"]["uid"] = "unknown-link" + response = self._post_webhook(payload) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json(), {"ok": True, "reconciled": False}) + self.tx.invalidate_recordset() + self.assertEqual(self.tx.state, "pending") + + def test_malformed_webhooks_do_not_modify_transaction(self): + for body in (b"{", b"\xff", b"[]", b'{"session":"complete"}', b"{}"): + with self.subTest(body=body): + response = self.opener.post( + self._build_url(MakePayController._webhook_url), data=body, + headers={"Content-Type": "application/json"}, + ) + self.assertEqual(response.status_code, 400) + self.tx.invalidate_recordset() + self.assertEqual(self.tx.state, "pending") + + def test_signed_failed_and_canceled_webhooks_reconcile(self): + for status, state in (("failed", "error"), ("cancelled", "cancel"), ("paid", "done")): + with self.subTest(status=status): + self.tx.state = "pending" + response = self._post_webhook(self._payload(status)) + self.assertEqual(response.status_code, 200) + self.tx.invalidate_recordset() + self.assertEqual(self.tx.state, state) + + def test_event_substrings_do_not_confirm_payment(self): + payload = self._payload("pending") + payload["type"] = "makepay.payment.paid.failed" + response = self._post_webhook(payload) + self.assertEqual(response.status_code, 200) + self.tx.invalidate_recordset() + self.assertEqual(self.tx.state, "pending") + + def test_checkout_creates_and_renders_hosted_link(self): + response = {"uid": "checkout-link", "checkoutUrl": "https://www.makepay.io/payment/checkout-link"} + with patch.object(type(self.provider), "_makepay_create_payment_link", return_value=response) as create_link: + processing_values = self.tx._get_processing_values() + payload = create_link.call_args.args[0] + self.assertEqual(payload["orderId"], self.tx.reference) + self.assertNotIn("status=", payload["successUrl"]) + self.assertNotIn("status=", payload["failureUrl"]) + self.assertIn(MakePayController._return_url, payload["returnUrl"]) + self.assertIn(response["checkoutUrl"], str(processing_values["redirect_form_html"])) + self.assertEqual(self.tx.makepay_payment_link_uid, "checkout-link") + self.assertEqual(self.tx.provider_reference, "checkout-link") + self.assertEqual(self.tx.state, "pending")