From 4e61e9b1ab29273ce4344d3643988188fa32fadb Mon Sep 17 00:00:00 2001 From: Deeman Date: Wed, 18 Feb 2026 16:49:23 +0100 Subject: [PATCH] fix broken webhook signature verification and stale billing tests Webhook handler called Verifier().verify() with raw bytes instead of a request object, so signature verification always failed. Replaced with manual HMAC check matching Paddle's ts=...;h1=... format. Updated tests to produce correct signature format, mock the SDK instead of httpx for manage/cancel routes, and expect JSON for overlay checkout. Co-Authored-By: Claude Opus 4.6 --- CHANGELOG.md | 10 +++ padelnomics/src/padelnomics/billing/routes.py | 36 +++++++-- padelnomics/tests/conftest.py | 9 ++- padelnomics/tests/test_billing_routes.py | 76 ++++++++----------- padelnomics/tests/test_billing_webhooks.py | 12 +-- padelnomics/tests/test_phase0.py | 10 --- 6 files changed, 85 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd54077..23c7db7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). README for testing email flows without a verified domain ### Fixed +- **Webhook signature verification broken** — `Verifier().verify()` was called + with raw bytes instead of a request object, causing all signed webhooks to + fail with 400; replaced with manual HMAC verification matching Paddle's + `ts=;h1=` format; also added JSON parse error guard (400 instead + of 500 on malformed payloads) +- **Billing tests stale after SDK migration** — webhook tests used plain + HMAC instead of Paddle's `ts=...;h1=...` signature format; checkout tests + expected redirect instead of JSON overlay response; manage/cancel tests + mocked httpx instead of Paddle SDK; removed stale `PADDLE_PRICES` config + test (prices now in DB) - **Quote wizard state loss** — `_accumulated` hidden input used `"` attribute delimiters which broke on `tojson` output containing literal `"` characters; switched all 8 step templates to single-quote delimiters (`value='...'`) diff --git a/padelnomics/src/padelnomics/billing/routes.py b/padelnomics/src/padelnomics/billing/routes.py index a184626..80bb6c5 100644 --- a/padelnomics/src/padelnomics/billing/routes.py +++ b/padelnomics/src/padelnomics/billing/routes.py @@ -3,14 +3,16 @@ Billing domain: checkout, webhooks, subscription management. Payment provider: paddle """ +import hashlib +import hmac import json +import time from datetime import datetime from functools import wraps from pathlib import Path from paddle_billing import Client as PaddleClient from paddle_billing import Environment, Options -from paddle_billing.Notifications import Secret, Verifier from quart import Blueprint, flash, g, jsonify, redirect, render_template, request, session, url_for from ..auth.routes import login_required @@ -210,6 +212,29 @@ async def cancel(): return redirect(url_for("dashboard.settings")) +def _verify_paddle_signature(payload: bytes, signature_header: str, secret: str, + max_drift_seconds: int = 300) -> bool: + """Verify Paddle webhook signature (ts=;h1=).""" + parts = {} + for kv in signature_header.split(";"): + if "=" in kv: + k, v = kv.split("=", 1) + parts[k] = v + + ts = parts.get("ts", "") + h1 = parts.get("h1", "") + if not ts or not h1: + return False + + if abs(time.time() - int(ts)) > max_drift_seconds: + return False + + expected = hmac.new( + secret.encode(), f"{ts}:{payload.decode()}".encode(), hashlib.sha256, + ).hexdigest() + return hmac.compare_digest(expected, h1) + + @bp.route("/webhook/paddle", methods=["POST"]) async def webhook(): """Handle Paddle webhooks.""" @@ -217,12 +242,13 @@ async def webhook(): sig = request.headers.get("Paddle-Signature", "") if config.PADDLE_WEBHOOK_SECRET: - try: - Verifier().verify(payload, Secret(config.PADDLE_WEBHOOK_SECRET), sig) - except Exception: + if not _verify_paddle_signature(payload, sig, config.PADDLE_WEBHOOK_SECRET): return jsonify({"error": "Invalid signature"}), 400 - event = json.loads(payload) + try: + event = json.loads(payload) + except (json.JSONDecodeError, ValueError): + return jsonify({"error": "Invalid JSON payload"}), 400 event_type = event.get("event_type") data = event.get("data", {}) custom_data = data.get("custom_data", {}) diff --git a/padelnomics/tests/conftest.py b/padelnomics/tests/conftest.py index e406106..f7ea86d 100644 --- a/padelnomics/tests/conftest.py +++ b/padelnomics/tests/conftest.py @@ -3,6 +3,7 @@ Shared test fixtures for the Padelnomics test suite. """ import hashlib import hmac +import time from datetime import datetime from pathlib import Path from unittest.mock import AsyncMock, patch @@ -118,7 +119,6 @@ def patch_config(): test_values = { "PADDLE_API_KEY": "test_api_key_123", "PADDLE_WEBHOOK_SECRET": "whsec_test_secret", - "PADDLE_PRICES": {"starter": "pri_starter_123", "pro": "pri_pro_456"}, "BASE_URL": "http://localhost:5000", "DEBUG": True, } @@ -160,5 +160,8 @@ def make_webhook_payload( def sign_payload(payload_bytes: bytes, secret: str = "whsec_test_secret") -> str: - """Compute HMAC-SHA256 signature for a webhook payload.""" - return hmac.new(secret.encode(), payload_bytes, hashlib.sha256).hexdigest() + """Build a Paddle-format signature header: ts=;h1=.""" + ts = str(int(time.time())) + data = f"{ts}:{payload_bytes.decode()}".encode() + h1 = hmac.new(secret.encode(), data, hashlib.sha256).hexdigest() + return f"ts={ts};h1={h1}" diff --git a/padelnomics/tests/test_billing_routes.py b/padelnomics/tests/test_billing_routes.py index 7cd46d4..d1c6fa7 100644 --- a/padelnomics/tests/test_billing_routes.py +++ b/padelnomics/tests/test_billing_routes.py @@ -1,12 +1,11 @@ """ Route integration tests for Paddle billing endpoints. -External Paddle API calls mocked with respx. +Checkout uses Paddle.js overlay (returns JSON), manage/cancel use Paddle SDK. """ -import httpx -import pytest -import respx +from unittest.mock import MagicMock, patch + +import pytest -CHECKOUT_METHOD = "POST" CHECKOUT_PLAN = "starter" @@ -40,7 +39,7 @@ class TestSuccessPage: # ════════════════════════════════════════════════════════════ -# Checkout +# Checkout (Paddle.js overlay — returns JSON) # ════════════════════════════════════════════════════════════ class TestCheckoutRoute: @@ -48,31 +47,25 @@ class TestCheckoutRoute: response = await client.post(f"/billing/checkout/{CHECKOUT_PLAN}", follow_redirects=False) assert response.status_code in (302, 303, 307) - @respx.mock - async def test_creates_checkout_session(self, auth_client, db, test_user): - respx.post("https://api.paddle.com/transactions").mock( - return_value=httpx.Response(200, json={ - "data": { - "checkout": { - "url": "https://checkout.paddle.com/test_123" - } - } - }) + async def test_returns_checkout_json(self, auth_client, db, test_user): + # Insert a paddle_products row so get_paddle_price() finds it + await db.execute( + "INSERT INTO paddle_products (key, paddle_product_id, paddle_price_id, name, price_cents, currency, billing_type) VALUES (?, ?, ?, ?, ?, ?, ?)", + ("starter", "pro_test", "pri_starter_123", "Starter", 1900, "EUR", "subscription"), ) - response = await auth_client.post(f"/billing/checkout/{CHECKOUT_PLAN}", follow_redirects=False) - assert response.status_code in (302, 303, 307) + await db.commit() + + response = await auth_client.post(f"/billing/checkout/{CHECKOUT_PLAN}") + assert response.status_code == 200 + data = await response.get_json() + assert "items" in data + assert data["items"][0]["priceId"] == "pri_starter_123" async def test_invalid_plan_rejected(self, auth_client, db, test_user): - response = await auth_client.post("/billing/checkout/invalid", follow_redirects=False) - assert response.status_code in (302, 303, 307) - - @respx.mock - async def test_api_error_propagates(self, auth_client, db, test_user): - respx.post("https://api.paddle.com/transactions").mock( - return_value=httpx.Response(500, json={"error": "server error"}) - ) - with pytest.raises(httpx.HTTPStatusError): - await auth_client.post(f"/billing/checkout/{CHECKOUT_PLAN}") + response = await auth_client.post("/billing/checkout/nonexistent_plan") + assert response.status_code == 400 + data = await response.get_json() + assert "error" in data # ════════════════════════════════════════════════════════════ @@ -88,20 +81,16 @@ class TestManageRoute: response = await auth_client.post("/billing/manage", follow_redirects=False) assert response.status_code in (302, 303, 307) - @respx.mock async def test_redirects_to_portal(self, auth_client, db, test_user, create_subscription): await create_subscription(test_user["id"], paddle_subscription_id="sub_test") - respx.get("https://api.paddle.com/subscriptions/sub_test").mock( - return_value=httpx.Response(200, json={ - "data": { - "management_urls": { - "update_payment_method": "https://paddle.com/manage/test_123" - } - } - }) - ) - response = await auth_client.post("/billing/manage", follow_redirects=False) + mock_sub = MagicMock() + mock_sub.management_urls.update_payment_method = "https://paddle.com/manage/test_123" + mock_client = MagicMock() + mock_client.subscriptions.get.return_value = mock_sub + + with patch("padelnomics.billing.routes._paddle_client", return_value=mock_client): + response = await auth_client.post("/billing/manage", follow_redirects=False) assert response.status_code in (302, 303, 307) @@ -118,15 +107,14 @@ class TestCancelRoute: response = await auth_client.post("/billing/cancel", follow_redirects=False) assert response.status_code in (302, 303, 307) - @respx.mock async def test_cancels_subscription(self, auth_client, db, test_user, create_subscription): await create_subscription(test_user["id"], paddle_subscription_id="sub_test") - respx.post("https://api.paddle.com/subscriptions/sub_test/cancel").mock( - return_value=httpx.Response(200, json={"data": {}}) - ) - response = await auth_client.post("/billing/cancel", follow_redirects=False) + mock_client = MagicMock() + with patch("padelnomics.billing.routes._paddle_client", return_value=mock_client): + response = await auth_client.post("/billing/cancel", follow_redirects=False) assert response.status_code in (302, 303, 307) + mock_client.subscriptions.cancel.assert_called_once() # ════════════════════════════════════════════════════════════ diff --git a/padelnomics/tests/test_billing_webhooks.py b/padelnomics/tests/test_billing_webhooks.py index d3b44f5..c9cdcfd 100644 --- a/padelnomics/tests/test_billing_webhooks.py +++ b/padelnomics/tests/test_billing_webhooks.py @@ -71,12 +71,12 @@ class TestWebhookSignature: async def test_empty_payload_rejected(self, client, db): sig = sign_payload(b"") - with pytest.raises(Exception): # JSONDecodeError in TESTING mode - await client.post( - WEBHOOK_PATH, - data=b"", - headers={SIG_HEADER: sig, "Content-Type": "application/json"}, - ) + response = await client.post( + WEBHOOK_PATH, + data=b"", + headers={SIG_HEADER: sig, "Content-Type": "application/json"}, + ) + assert response.status_code == 400 # ════════════════════════════════════════════════════════════ diff --git a/padelnomics/tests/test_phase0.py b/padelnomics/tests/test_phase0.py index 4adb043..820b937 100644 --- a/padelnomics/tests/test_phase0.py +++ b/padelnomics/tests/test_phase0.py @@ -691,13 +691,3 @@ class TestSchema: row = await cur.fetchone() assert row[0] is None - -# ════════════════════════════════════════════════════════════ -# Business plan price in config -# ════════════════════════════════════════════════════════════ - -class TestBusinessPlanConfig: - def test_business_plan_in_paddle_prices(self): - from padelnomics.core import Config - c = Config() - assert "business_plan" in c.PADDLE_PRICES