From 5dd73782abcf86c56673671fda420330d0726e8f Mon Sep 17 00:00:00 2001 From: Nicola Date: Tue, 29 Sep 2026 10:25:47 +0200 Subject: [PATCH] fix(security): Nexi redirect MAC is mandatory and compared in constant time (#3) handle_redirect checked the MAC only when a non-empty one was present, so ?codTrans=X&esito=OK completed a payment without the MAC key. A missing or empty mac now fails ("MAC missing"), a wrong one fails ("MAC verification failed"), and the comparison uses hmac.compare_digest on bytes. Release 0.1.1. Fixes #3 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 11 ++++ README.md | 2 + awesome_python_checkout/providers/nexi.py | 16 +++++- pyproject.toml | 2 +- tests/test_configurator.py | 19 ++++++ tests/test_providers.py | 70 ++++++++++++++++++++++- 6 files changed, 117 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b83f44..5f334f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.1.1] - 2026-09-29 + +### Security + +- **Nexi redirect MAC is mandatory** ([#3](https://github.com/nik2208/awesome-python-checkout/issues/3)). `NexiProvider.handle_redirect` used to check the MAC only when the query carried a non-empty one, so `GET /nexi/redirect?codTrans=X&esito=OK` completed a payment without knowing `mac_key`. + - A missing or empty `mac` now returns `status="failed"` with `raw={"error": "MAC missing"}`. + - A wrong `mac` returns `raw={"error": "MAC verification failed"}`. + - The comparison uses `hmac.compare_digest` on bytes, so it runs in constant time and a non-ASCII `mac` does not raise. + - `CheckoutConfigurator.handle_redirect` emits `payment.failed` for these outcomes, never `payment.completed`. + - Compatibility: an `esito=OK` redirect without a valid MAC goes from `completed` to `failed`. + ## [0.1.0] - 2026-05-06 ### Added diff --git a/README.md b/README.md index 2cebd0a..5ce9ec7 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,8 @@ app.include_router(checkout.router(), prefix="/checkout") | `mac_key` | `str` | required | Nexi MAC key for SHA-1 signature | | `environment` | `"sandbox" \| "live"` | `"sandbox"` | Target environment | +The redirect outcome MAC is **required** and is compared in constant time (`hmac.compare_digest`). A redirect without `mac`, with an empty one or with a wrong one returns `status="failed"` (`raw={"error": "MAC missing"}` or `{"error": "MAC verification failed"}`) and emits `payment.failed`. It never returns `completed`, whatever `esito` says. + ### SatispayConfig | Field | Type | Default | Description | diff --git a/awesome_python_checkout/providers/nexi.py b/awesome_python_checkout/providers/nexi.py index 9e082ee..3aa6dd4 100644 --- a/awesome_python_checkout/providers/nexi.py +++ b/awesome_python_checkout/providers/nexi.py @@ -3,6 +3,7 @@ from __future__ import annotations import hashlib +import hmac import urllib.parse from dataclasses import dataclass from typing import Any, Literal @@ -174,7 +175,20 @@ async def handle_redirect(self, query: dict[str, str]) -> PaymentResult: } ) - if received_mac and received_mac.lower() != expected_mac.lower(): + # The outcome MAC is mandatory: without it anyone could forge + # ``esito=OK``. Compare in constant time, as bytes: hmac.compare_digest + # raises TypeError on non-ASCII ``str``. ``expected_mac`` is a + # hashlib hexdigest, already lowercase. + if not received_mac: + return PaymentResult( + payment_id=cod_trans, + status="failed", + provider=self.name, + raw={"error": "MAC missing"}, + ) + if not hmac.compare_digest( + received_mac.lower().encode("utf-8"), expected_mac.encode("utf-8") + ): return PaymentResult( payment_id=cod_trans, status="failed", diff --git a/pyproject.toml b/pyproject.toml index aa7ee80..1ca71e1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "awesome-python-checkout" -version = "0.1.0" +version = "0.1.1" description = "FastAPI payment checkout library — Python port of awesome-node-checkout" readme = "README.md" license = { text = "MIT" } diff --git a/tests/test_configurator.py b/tests/test_configurator.py index 274c6a4..b445d1e 100644 --- a/tests/test_configurator.py +++ b/tests/test_configurator.py @@ -175,6 +175,25 @@ async def test_handle_redirect_fires_completed_event(): assert "completed" in events +@pytest.mark.asyncio +async def test_handle_redirect_nexi_missing_mac_fires_failed_event(): + from awesome_python_checkout import NexiProvider + from awesome_python_checkout.providers import NexiConfig + + checkout = CheckoutConfigurator() + checkout.register_provider(NexiProvider(NexiConfig(merchant_id="M", mac_key="k"))) + completed: list[Any] = [] + failed: list[Any] = [] + checkout.on("payment.completed", completed.append) + checkout.on("payment.failed", failed.append) + + result = await checkout.handle_redirect("nexi", {"codTrans": "ORD-1", "esito": "OK"}) + + assert result.status == "failed" + assert completed == [] + assert failed == [{"provider": "nexi", "error": {"error": "MAC missing"}}] + + # --------------------------------------------------------------------------- # InMemoryTransactionStore # --------------------------------------------------------------------------- diff --git a/tests/test_providers.py b/tests/test_providers.py index 798117f..c212770 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -193,10 +193,77 @@ async def test_nexi_handle_redirect_ok(nexi_provider: NexiProvider): assert result.payment_id == "ORD-001" +def _nexi_signed_outcome(**overrides: str) -> dict[str, str]: + """Redirect outcome signed with the ``nexi_provider`` fixture's MAC key.""" + import hashlib + + fields = { + "codTrans": "ORD-001", + "esito": "OK", + "importo": "4999", + "divisa": "EUR", + "data": "20260101", + "orario": "120000", + "codAut": "AUTH123", + } + fields.update(overrides) + mac_string = ( + f"codTrans={fields['codTrans']}" + f"esito={fields['esito']}" + f"importo={fields['importo']}" + f"divisa={fields['divisa']}" + f"data={fields['data']}" + f"orario={fields['orario']}" + f"codAut={fields['codAut']}" + "secret-mac-key" + ) + fields["mac"] = hashlib.sha1(mac_string.encode()).hexdigest() + return fields + + @pytest.mark.asyncio async def test_nexi_handle_redirect_failed_esito(nexi_provider: NexiProvider): - result = await nexi_provider.handle_redirect({"esito": "KO", "codTrans": "ORD-002", "mac": ""}) + # Valid MAC, so this exercises the esito path and not the MAC checks. + result = await nexi_provider.handle_redirect( + _nexi_signed_outcome(codTrans="ORD-002", esito="KO") + ) assert result.status == "failed" + assert result.payment_id == "ORD-002" + assert result.raw.get("esito") == "KO" + + +@pytest.mark.asyncio +async def test_nexi_handle_redirect_missing_mac_rejected(nexi_provider: NexiProvider): + result = await nexi_provider.handle_redirect({"codTrans": "ORD-1", "esito": "OK"}) + assert result.status == "failed" + assert result.payment_id == "ORD-1" + assert result.raw == {"error": "MAC missing"} + + +@pytest.mark.asyncio +async def test_nexi_handle_redirect_empty_mac_rejected(nexi_provider: NexiProvider): + result = await nexi_provider.handle_redirect({"codTrans": "ORD-1", "esito": "OK", "mac": ""}) + assert result.status == "failed" + assert result.payment_id == "ORD-1" + assert result.raw == {"error": "MAC missing"} + + +@pytest.mark.asyncio +async def test_nexi_handle_redirect_non_ascii_mac_rejected(nexi_provider: NexiProvider): + outcome = _nexi_signed_outcome() + outcome["mac"] = "é" * 40 + result = await nexi_provider.handle_redirect(outcome) + assert result.status == "failed" + assert result.raw == {"error": "MAC verification failed"} + + +@pytest.mark.asyncio +async def test_nexi_handle_redirect_uppercase_mac_accepted(nexi_provider: NexiProvider): + outcome = _nexi_signed_outcome() + outcome["mac"] = outcome["mac"].upper() + result = await nexi_provider.handle_redirect(outcome) + assert result.status == "completed" + assert result.payment_id == "ORD-001" @pytest.mark.asyncio @@ -214,6 +281,7 @@ async def test_nexi_handle_redirect_bad_mac(nexi_provider: NexiProvider): } ) assert result.status == "failed" + assert result.raw == {"error": "MAC verification failed"} @pytest.mark.asyncio