diff --git a/app/routers/stripe_billing.py b/app/routers/stripe_billing.py index 77edb8c..30ab079 100644 --- a/app/routers/stripe_billing.py +++ b/app/routers/stripe_billing.py @@ -142,6 +142,14 @@ async def create_checkout( # Lets us paste in a referral coupon at checkout once the # referral redemption flow ships. "allow_promotion_codes": True, + # Collect a billing address on every checkout so each Stripe + # Customer carries a country. Two reasons: card-fraud checks get + # materially better with AVS data, and EU B2C digital-services + # VAT is charged at the *consumer's* place of supply — we can't + # answer the OSS question at all without knowing where buyers + # are. Note this is the address on the card, not an IP guess, + # which is the evidence a tax authority actually accepts. + "billing_address_collection": "required", } # No `currency` kwarg — every checkout bills the Price's base # currency (GBP), matching the static £7 / £70 copy on /pricing. @@ -165,6 +173,12 @@ async def create_checkout( create_kwargs["subscription_data"] = {"trial_period_days": 14} if user.stripe_customer_id: create_kwargs["customer"] = user.stripe_customer_id + # Required for billing_address_collection to actually persist: + # when `customer` is supplied, Stripe collects the address for + # the payment but leaves the Customer record untouched unless + # customer_update.address is "auto". Without this the country + # lands on the PaymentIntent and nowhere durable. + create_kwargs["customer_update"] = {"address": "auto"} else: create_kwargs["customer_email"] = user.email @@ -294,11 +308,15 @@ async def _grant_paid( await convert_referral(session, user) -async def _revoke_paid(user: User) -> None: +async def _revoke_paid(user: User, *, keep_subscription: bool = False) -> None: user.tier = "free" - user.stripe_subscription_id = None + if not keep_subscription: + user.stripe_subscription_id = None user.stripe_trial_end_at = None # Keep stripe_customer_id so a re-subscription matches this row. + # `keep_subscription` is for a pause: the subscription still exists + # at Stripe and will resume under the same id, so nulling our copy + # would lose the link while access is merely suspended. async def _handle_checkout_completed( @@ -336,6 +354,16 @@ async def _handle_subscription_event( customer_id=obj.get("customer")) return status = obj.get("status") + # `pause_collection` is a *different* mechanism from status="paused": + # the subscription stays `active` while Stripe simply stops invoicing. + # Unhandled, that leaves the customer on paid features indefinitely + # without paying, so treat any live pause as not-paid regardless of + # status. Pause is disabled in our live portal configuration, so in + # practice this only fires if someone re-enables it there or pauses + # from the Dashboard — which is exactly when we'd want it to work. + if obj.get("pause_collection"): + await _revoke_paid(user, keep_subscription=True) + return # Stripe statuses: trialing, active, past_due, canceled, unpaid, # incomplete, incomplete_expired, paused. Treat trialing/active as # paid; everything else holds tier the same until we get an explicit @@ -362,6 +390,22 @@ async def _handle_subscription_deleted( await _revoke_paid(user) +async def _handle_subscription_paused( + session: AsyncSession, event_type: str, obj: dict[str, Any], +) -> None: + """customer.subscription.paused — status flips to `paused` when a + trial ends with no usable payment method (trial_settings.end_behavior + .missing_payment_method = pause). No money is being collected, so + paid features come off. `.resumed` routes to the normal subscription + handler, which grants again on active/trialing.""" + user = await _find_user(session, customer_id=obj.get("customer")) + if user is None: + log.warning("stripe.user_not_found", event_type=event_type, + customer_id=obj.get("customer")) + return + await _revoke_paid(user, keep_subscription=True) + + async def _handle_audit_only( session: AsyncSession, event_type: str, obj: dict[str, Any], ) -> None: @@ -376,6 +420,8 @@ _HANDLERS = { "customer.subscription.created": _handle_subscription_event, "customer.subscription.updated": _handle_subscription_event, "customer.subscription.deleted": _handle_subscription_deleted, + "customer.subscription.paused": _handle_subscription_paused, + "customer.subscription.resumed": _handle_subscription_event, "invoice.paid": _handle_audit_only, "invoice.payment_failed": _handle_audit_only, "charge.refunded": _handle_audit_only, diff --git a/tests/test_stripe_billing.py b/tests/test_stripe_billing.py index 6feba1a..f592850 100644 --- a/tests/test_stripe_billing.py +++ b/tests/test_stripe_billing.py @@ -341,6 +341,131 @@ def test_subscription_active_grants_paid(tmp_path): assert asyncio.run(_check_tier()) == "paid" +# --- pause / resume -------------------------------------------------------- + + +def _activate(client, *, customer="cus_p", subscription="sub_p", evt="evt_a"): + """Link user 1 to a Stripe customer and put them on paid.""" + return _post_webhook(client, body={ + "id": evt, + "type": "checkout.session.completed", + "data": {"object": { + "client_reference_id": "1", + "customer": customer, + "subscription": subscription, + }}, + }) + + +def _tier_and_sub(factory): + async def _check(): + from sqlalchemy import select + from app.models import User + async with factory() as session: + u = (await session.execute( + select(User).where(User.id == 1) + )).scalar_one() + return u.tier, u.stripe_subscription_id + return asyncio.run(_check()) + + +def test_subscription_paused_drops_tier(tmp_path): + """status=paused means Stripe has stopped collecting (trial ended + with no usable card). Paid features must come off — otherwise the + customer keeps everything for free.""" + client, factory, _ = _build_app(tmp_path) + _activate(client) + assert _tier_and_sub(factory)[0] == "paid" + + r = _post_webhook(client, body={ + "id": "evt_paused", + "type": "customer.subscription.paused", + "data": {"object": { + "id": "sub_p", "customer": "cus_p", "status": "paused", + }}, + }) + assert r.status_code == 200, r.text + assert r.json()["status"] == "ok", "must not fall through to 'ignored'" + + tier, sub = _tier_and_sub(factory) + assert tier == "free" + # The subscription still exists at Stripe and resumes under the same + # id, so we keep our link to it. + assert sub == "sub_p" + + +def test_subscription_resumed_regrants_tier(tmp_path): + client, factory, _ = _build_app(tmp_path) + _activate(client) + _post_webhook(client, body={ + "id": "evt_paused2", + "type": "customer.subscription.paused", + "data": {"object": { + "id": "sub_p", "customer": "cus_p", "status": "paused", + }}, + }) + assert _tier_and_sub(factory)[0] == "free" + + r = _post_webhook(client, body={ + "id": "evt_resumed", + "type": "customer.subscription.resumed", + "data": {"object": { + "id": "sub_p", "customer": "cus_p", "status": "active", + }}, + }) + assert r.status_code == 200, r.text + assert r.json()["status"] == "ok" + assert _tier_and_sub(factory) == ("paid", "sub_p") + + +def test_pause_collection_drops_tier_despite_active_status(tmp_path): + """The portal's pause uses `pause_collection` and leaves status as + `active`, so the status check alone would keep the user on paid while + Stripe bills them nothing.""" + client, factory, _ = _build_app(tmp_path) + _activate(client) + assert _tier_and_sub(factory)[0] == "paid" + + r = _post_webhook(client, body={ + "id": "evt_pause_coll", + "type": "customer.subscription.updated", + "data": {"object": { + "id": "sub_p", + "customer": "cus_p", + "status": "active", + "pause_collection": {"behavior": "void"}, + }}, + }) + assert r.status_code == 200, r.text + assert _tier_and_sub(factory) == ("free", "sub_p") + + +def test_unpause_collection_regrants_tier(tmp_path): + """Resuming collection sends subscription.updated with + pause_collection cleared to null — that must grant paid back.""" + client, factory, _ = _build_app(tmp_path) + _activate(client) + _post_webhook(client, body={ + "id": "evt_pc_on", + "type": "customer.subscription.updated", + "data": {"object": { + "id": "sub_p", "customer": "cus_p", "status": "active", + "pause_collection": {"behavior": "void"}, + }}, + }) + assert _tier_and_sub(factory)[0] == "free" + + _post_webhook(client, body={ + "id": "evt_pc_off", + "type": "customer.subscription.updated", + "data": {"object": { + "id": "sub_p", "customer": "cus_p", "status": "active", + "pause_collection": None, + }}, + }) + assert _tier_and_sub(factory) == ("paid", "sub_p") + + # --- idempotency + unknown ------------------------------------------------ @@ -524,6 +649,30 @@ def test_checkout_rejects_currency_in_body(tmp_path): assert r.status_code == 200, r.text +def test_checkout_requires_billing_address(tmp_path): + """Every checkout must collect a billing address, so each Stripe + Customer ends up with a country. EU B2C digital-services VAT is due + at the consumer's place of supply, and the card's billing country is + the evidence for that — an IP guess is not.""" + client, _, session_cookie = _build_app(tmp_path) + + def asserter(params): + assert params["billing_address_collection"] == "required" + # New customer (no stored customer id): customer_update is only + # valid alongside `customer`, so it must be absent here. + assert "customer_update" not in params + + for cadence in ("monthly", "annual"): + with patch("app.routers.stripe_billing._stripe_client", + return_value=_fake_checkout_client(asserter)): + r = client.post( + "/api/stripe/checkout", + json={"cadence": cadence}, + cookies={"cassandra_session": session_cookie}, + ) + assert r.status_code == 200, r.text + + def test_checkout_uses_existing_customer_ref(tmp_path): """Existing customer: use the stored `customer` ref rather than `customer_email`, so repeat checkouts don't mint duplicate Stripe @@ -546,6 +695,10 @@ def test_checkout_uses_existing_customer_ref(tmp_path): assert "currency" not in params assert params["customer"] == "cus_existing_xxxxxxxxxxxxxx" assert "customer_email" not in params + # Without customer_update.address the collected address is + # attached to the payment only and the Customer record keeps a + # null address — i.e. still no country for the VAT question. + assert params["customer_update"] == {"address": "auto"} with patch("app.routers.stripe_billing._stripe_client", return_value=_fake_checkout_client(asserter)):