Merge pull request #2087 from Cedric921/fix/partial-refund-suspension

fix(license): suspend only the license a fully refunded charge paid for

docs/architecture/license/portal.md

@@ -49,7 +49,7 @@ The expiry is `max(now, LICENSE_TRIAL_EXPIRY_FLOOR) + LICENSE_TRIAL_DAYS`, 30 da
 `app/payments/webhook.py` verifies the signature and hands each paid checkout session to the handler that claims it (`app/payments/registry.py`); `app/license/purchase.py` registers the license's claims.
 
 - `checkout.session.completed` with `payment_status` `paid` or `no_payment_required`, and `checkout.session.async_payment_succeeded`: the license handler claims a session whose metadata says `purchase_type: license`, or, with no `purchase_type` and a license price configured, whose line item is a license price. It fulfils the session, then emails the file. A mail failure does not fail the webhook: the license exists, the success page serves it, and a Stripe retry would only risk a duplicate.
-- `charge.refunded`: lists licenses by `metadata[stripeCustomerId]` and suspends every one. Suspend, not revoke: it is reversible, the record stays listable for support, and Keygen's `validate-key` then reports `SUSPENDED`, which contract 2 maps to `revoked`.
+- `charge.refunded`: acts only on a full refund, `refunded` or `amount_refunded >= amount`; a partial refund suspends nothing. It resolves the charge's `payment_intent` to its checkout session through Stripe, that session to its one license by `checkoutSessionId`, and suspends that license alone, so a customer's other purchases keep working. A refund that traces to no single license suspends nothing and logs a warning for support, because a wrong suspension is silent to the customer. Failures are logged and the webhook still answers 200, since a retry would re-run the suspension. Suspend, not revoke: it is reversible, the record stays listable for support, and Keygen's `validate-key` then reports `SUSPENDED`, which contract 2 maps to `revoked`.
 
 Fulfilment also writes the license's Keygen id onto the Stripe customer as `keygen_license_id`, best effort, so a Stripe error cannot fail the webhook. Nothing reads it back; refunds find licenses through Keygen metadata.
 
@@ -64,8 +64,8 @@ Fulfilment also writes the license's Keygen id onto the Stripe customer as `keyg
 | `plan` | every license | the app (contract 1) |
 | `email` | every license | delivery, resend, shown in Settings |
 | `trialKey` | trials | the one-trial-per-person check |
-| `stripeCustomerId` | Stripe purchases | refund → suspend |
-| `checkoutSessionId` | Stripe purchases | the success page, support corrections |
+| `stripeCustomerId` | Stripe purchases | the Stripe customer a refund comes from |
+| `checkoutSessionId` | Stripe purchases | the success page, support corrections, refund → suspend |
 
 Keygen camelCases metadata keys in filter queries, and a misspelled filter returns an empty list rather than an error, which reads as "no license". The spellings live in `app/license/models.py`, except that `create_license()` in `keygen.py` writes `plan` and `email` as literals.
 
@@ -169,5 +169,4 @@ A trial has no payment to prove, so the anchor is the address itself: support as
 
 ## Known gaps
 
-- A refund suspends every license the Stripe customer holds, partial refunds included.
 - No test covers the license routes beyond their mail check, or their rate limits, and no contract test runs the Keygen client against a real Keygen.

surfsense_backend/app/license/purchase.py

@@ -17,7 +17,8 @@ from .admin import suspend_licenses_for_customer
 from .checkout import resolve_license_plan
 from .email.deliver import deliver_licenses
 from .issue import fulfill_license_session
-from .models import LicenseIssueError
+from .models import LicenseIssueError, LicenseNotFoundError
+from .records import find_license_by_checkout_session
 
 logger = logging.getLogger(__name__)
 
@@ -77,36 +78,79 @@ async def _fulfill(
     return StripeWebhookResponse()
 
 
-async def _suspend_refunded(charge: Any, **_: Any) -> StripeWebhookResponse:
-    """Suspend a refunded buyer's licenses.
+async def _suspend_refunded(
+    charge: Any, *, stripe_client: Any = None, **_: Any
+) -> StripeWebhookResponse:
+    """Suspend the licence a fully refunded charge paid for, and no other.
 
     Suspend rather than revoke: reversible, still listable for support, and
     ``validate-key`` then reports SUSPENDED, which contract 2 maps to the
-    ``revoked`` reason.
+    ``revoked`` reason. A partial refund suspends nothing: it is a goodwill or
+    seat adjustment, and the rest of the purchase is still paid for.
     """
     customer = getattr(charge, "customer", None)
     customer_id = (
         customer if isinstance(customer, str) else getattr(customer, "id", None)
     )
-    if not customer_id:
+    if not customer_id or not _fully_refunded(charge):
         return StripeWebhookResponse()
 
     try:
-        suspended = await suspend_licenses_for_customer(str(customer_id))
+        license_id = await _license_paid_by(charge, stripe_client)
+        if license_id is None:
+            # A wrong suspension is silent (resend skips suspended licences), so
+            # a refund that cannot be traced to one licence is left to support.
+            logger.warning(
+                "Refund for customer %s traces to no single license; not suspending",
+                customer_id,
+            )
+            return StripeWebhookResponse()
+        await suspend_licenses_for_customer(
+            str(customer_id), keygen_license_id=license_id
+        )
     except Exception:
+        # Swallowed on purpose: a 500 makes Stripe retry, and a retry would
+        # re-run the suspension.
         logger.exception(
             "Could not suspend licenses for refunded customer %s", customer_id
         )
         return StripeWebhookResponse()
 
-    if suspended:
     logger.info(
-            "Suspended %d license(s) after refund for customer %s",
-            suspended,
-            customer_id,
+        "Suspended license %s after refund for customer %s", license_id, customer_id
     )
     return StripeWebhookResponse()
 
 
+def _fully_refunded(charge: Any) -> bool:
+    if getattr(charge, "refunded", False):
+        return True
+    amount = getattr(charge, "amount", None)
+    refunded = getattr(charge, "amount_refunded", None)
+    return isinstance(amount, int) and isinstance(refunded, int) and refunded >= amount
+
+
+async def _license_paid_by(charge: Any, stripe_client: Any) -> str | None:
+    """The one licence the charge's checkout session issued, through Stripe."""
+    payment_intent = getattr(charge, "payment_intent", None)
+    payment_intent_id = (
+        payment_intent
+        if isinstance(payment_intent, str)
+        else getattr(payment_intent, "id", None)
+    )
+    if not payment_intent_id or stripe_client is None:
+        return None
+    sessions = stripe_client.v1.checkout.sessions.list(
+        params={"payment_intent": payment_intent_id, "limit": 1}
+    )
+    if not sessions.data:
+        return None
+    try:
+        record = await find_license_by_checkout_session(sessions.data[0].id)
+    except LicenseNotFoundError:
+        return None
+    return record.keygen_license_id or None
+
+
 registry.claims_checkout("license", _claims_checkout, _fulfill)
 registry.on_event("charge.refunded", _suspend_refunded)

surfsense_backend/tests/unit/license/test_refund_suspension.py

@@ -0,0 +1,103 @@
+"""A refund suspends the licence that charge paid for, and only when fully refunded.
+
+The webhook used to suspend every licence the Stripe customer held, so a partial
+refund, or a refund of one of two purchases, silently disabled a licence the
+customer still paid for.
+"""
+
+from __future__ import annotations
+
+from types import SimpleNamespace
+
+import pytest
+
+import app.license.purchase  # noqa: F401  registers the refund handler
+from app.license import keygen
+from app.license.models import META_CUSTOMER, META_SESSION
+from app.payments import registry
+
+pytestmark = pytest.mark.unit
+
+HELD = {"cs_first": "lic-first", "cs_second": "lic-second"}
+
+
+def charge(amount_refunded: int, *, amount: int = 4900) -> SimpleNamespace:
+    """A charge.refunded object for the second of the customer's two purchases."""
+    return SimpleNamespace(
+        customer="cus_1",
+        amount=amount,
+        amount_refunded=amount_refunded,
+        refunded=amount_refunded >= amount,
+        payment_intent="pi_second",
+    )
+
+
+def stripe_client(sessions: dict[str, str]) -> SimpleNamespace:
+    """Stripe's checkout session list, keyed by payment intent."""
+
+    def list_sessions(params: dict) -> SimpleNamespace:
+        found = sessions.get(params["payment_intent"])
+        return SimpleNamespace(data=[SimpleNamespace(id=found)] if found else [])
+
+    sessions_api = SimpleNamespace(list=list_sessions)
+    return SimpleNamespace(
+        v1=SimpleNamespace(checkout=SimpleNamespace(sessions=sessions_api))
+    )
+
+
+@pytest.fixture
+def suspended(monkeypatch) -> list[str]:
+    """A customer holding two licences in Keygen; records what gets suspended."""
+    calls: list[str] = []
+
+    async def list_licenses(*, metadata=None, limit=100, **_):
+        metadata = metadata or {}
+        if META_SESSION in metadata:
+            license_id = HELD.get(metadata[META_SESSION])
+            return (
+                [{"id": license_id, "attributes": {"metadata": {}}}]
+                if license_id
+                else []
+            )
+        if metadata.get(META_CUSTOMER) == "cus_1":
+            return [
+                {"id": lid, "attributes": {"metadata": {}}} for lid in HELD.values()
+            ]
+        return []
+
+    async def suspend_license(license_id: str) -> None:
+        calls.append(license_id)
+
+    monkeypatch.setattr(keygen, "list_licenses", list_licenses)
+    monkeypatch.setattr(keygen, "suspend_license", suspend_license)
+    return calls
+
+
+async def refund(obj: SimpleNamespace, sessions: dict[str, str]) -> None:
+    handler = registry.event_handler("charge.refunded")
+    await handler(obj, db_session=None, stripe_client=stripe_client(sessions))
+
+
+async def test_a_partial_refund_suspends_nothing(suspended) -> None:
+    """A goodwill refund leaves the customer a licence they still paid for."""
+    await refund(charge(amount_refunded=1000), {"pi_second": "cs_second"})
+
+    assert suspended == []
+
+
+async def test_a_full_refund_suspends_only_the_licence_that_charge_bought(
+    suspended,
+) -> None:
+    """The customer's other purchase keeps working."""
+    await refund(charge(amount_refunded=4900), {"pi_second": "cs_second"})
+
+    assert suspended == ["lic-second"]
+
+
+async def test_a_refund_no_licence_can_be_traced_to_suspends_nothing(
+    suspended,
+) -> None:
+    """Guessing would disable a licence silently; support gets a log line instead."""
+    await refund(charge(amount_refunded=4900), {})
+
+    assert suspended == []