diff --git a/docs/architecture/license/portal.md b/docs/architecture/license/portal.md index 61fcd57a8..e13b0d6dc 100644 --- a/docs/architecture/license/portal.md +++ b/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. diff --git a/surfsense_backend/app/license/purchase.py b/surfsense_backend/app/license/purchase.py index a158bec8e..90bbaba93 100644 --- a/surfsense_backend/app/license/purchase.py +++ b/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, - ) + logger.info( + "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) diff --git a/surfsense_backend/tests/unit/license/test_refund_suspension.py b/surfsense_backend/tests/unit/license/test_refund_suspension.py new file mode 100644 index 000000000..b1aee9ad0 --- /dev/null +++ b/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 == []